From ce70ca1a9fa5c98381a1239e49922d8f6332c503 Mon Sep 17 00:00:00 2001 From: Jacob Alber Date: Thu, 7 May 2026 14:36:15 -0400 Subject: [PATCH 1/6] .NET: feat: Implement Magentic Orchestration for .NET (#5595) * feat: Implement Magentic Orchestration for .NET * fixup: Update for review comments * fix: Fix FenceJsonRegexPattern * fix: Format * fix: Updates for PR feedback * fix: Add missing serialized types to source gen for trimming * fix: Address PR Comments --- .../ExecutorEvent.cs | 2 + .../MagenticPlanReviewRequest.cs | 44 +++ .../MagenticPlanReviewResponse.cs | 18 + .../MagenticProgressLedger.cs | 269 +++++++++++++++ .../MagenticWorkflowBuilder.cs | 158 +++++++++ .../ResetChatSignal.cs | 9 + .../Specialized/AIAgentHostExecutor.cs | 15 +- .../Magentic/ChatMessageExtensions.cs | 175 ++++++++++ .../Magentic/ExecutorAgentHarness.cs | 72 ++++ .../Specialized/Magentic/MagenticConstants.cs | 8 + .../Specialized/Magentic/MagenticManager.cs | 122 +++++++ .../Magentic/MagenticOrchestrator.cs | 326 ++++++++++++++++++ .../Magentic/MagenticTaskContext.cs | 95 +++++ .../Specialized/Magentic/PromptTemplates.cs | 151 ++++++++ .../StreamingToolCallResultPairMatcher.cs | 77 +++++ .../WorkflowEvent.cs | 3 + .../WorkflowsJsonUtilities.cs | 5 + .../MagenticManagerTests.cs | 153 ++++++++ .../MagenticProgressLedgerTests.cs | 214 ++++++++++++ .../TestProgressLedgerState.cs | 88 +++++ .../TestReplayAgent.cs | 59 ++-- 21 files changed, 2038 insertions(+), 25 deletions(-) create mode 100644 dotnet/src/Microsoft.Agents.AI.Workflows/MagenticPlanReviewRequest.cs create mode 100644 dotnet/src/Microsoft.Agents.AI.Workflows/MagenticPlanReviewResponse.cs create mode 100644 dotnet/src/Microsoft.Agents.AI.Workflows/MagenticProgressLedger.cs create mode 100644 dotnet/src/Microsoft.Agents.AI.Workflows/MagenticWorkflowBuilder.cs create mode 100644 dotnet/src/Microsoft.Agents.AI.Workflows/ResetChatSignal.cs create mode 100644 dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/ChatMessageExtensions.cs create mode 100644 dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/ExecutorAgentHarness.cs create mode 100644 dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticConstants.cs create mode 100644 dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticManager.cs create mode 100644 dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticOrchestrator.cs create mode 100644 dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticTaskContext.cs create mode 100644 dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/PromptTemplates.cs create mode 100644 dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/StreamingToolCallResultPairMatcher.cs create mode 100644 dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/MagenticManagerTests.cs create mode 100644 dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/MagenticProgressLedgerTests.cs create mode 100644 dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/TestProgressLedgerState.cs diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/ExecutorEvent.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/ExecutorEvent.cs index a0d4dd73b4..3d590ea571 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows/ExecutorEvent.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/ExecutorEvent.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. using System.Text.Json.Serialization; +using Microsoft.Agents.AI.Workflows.Specialized.Magentic; namespace Microsoft.Agents.AI.Workflows; @@ -10,6 +11,7 @@ namespace Microsoft.Agents.AI.Workflows; [JsonDerivedType(typeof(ExecutorInvokedEvent))] [JsonDerivedType(typeof(ExecutorCompletedEvent))] [JsonDerivedType(typeof(ExecutorFailedEvent))] +[JsonDerivedType(typeof(MagenticOrchestratorEvent))] public class ExecutorEvent(string executorId, object? data) : WorkflowEvent(data) { /// diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/MagenticPlanReviewRequest.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/MagenticPlanReviewRequest.cs new file mode 100644 index 0000000000..fd7b82afe6 --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/MagenticPlanReviewRequest.cs @@ -0,0 +1,44 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.Collections.Generic; +using System.Linq; +using Microsoft.Extensions.AI; + +namespace Microsoft.Agents.AI.Workflows; + +/// +/// Request for human review of a proposed plan. +/// +/// The proposed plan. +/// The current progress ledger, if available. During the initial plan review, +/// this will be . In subsequent reviews after replanning (due to stalls), this will +/// contain the latest progress ledger that determined that no progress has been made or the workflow was in +/// a loop. +/// Whether the workflow is currently stalled. +public record MagenticPlanReviewRequest(ChatMessage Plan, MagenticProgressLedger? CurrentProgress, bool IsStalled) +{ + /// + /// Create an approving . + /// + /// + public MagenticPlanReviewResponse Approve() => new([]); + + /// + /// Create a with revisions. + /// + /// + public MagenticPlanReviewResponse Revise(string message) => new([new(ChatRole.User, message)]); + + /// + /// Create a with revisions. + /// + /// + public MagenticPlanReviewResponse Revise(ChatMessage message) => new([message]); + + /// + /// Create a with revisions. + /// + /// + public MagenticPlanReviewResponse Revise(IEnumerable messages) + => new(messages is List messageList ? messageList : messages.ToList()); +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/MagenticPlanReviewResponse.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/MagenticPlanReviewResponse.cs new file mode 100644 index 0000000000..0a72ccfa0f --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/MagenticPlanReviewResponse.cs @@ -0,0 +1,18 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.Collections.Generic; +using Microsoft.Extensions.AI; + +namespace Microsoft.Agents.AI.Workflows; + +/// +/// Review feedback for a proposed plan, including any revisions if the plan is not approved as-is. An +/// empty list of review messages indicates approval of the proposed plan without any revisions. +/// +/// +/// Review feedback for a generated plan. Empty if the plan is approved as-is and changes are requested. +/// +public record MagenticPlanReviewResponse(List Review) +{ + internal bool IsApproved => this.Review.Count == 0; +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/MagenticProgressLedger.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/MagenticProgressLedger.cs new file mode 100644 index 0000000000..445007d3f3 --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/MagenticProgressLedger.cs @@ -0,0 +1,269 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Linq; +using System.Text; +using System.Text.Json; +using System.Text.Json.Serialization; +using System.Text.Json.Serialization.Metadata; + +namespace Microsoft.Agents.AI.Workflows; + +/// +/// Maintains a ledger of progress made by the Magentic workflow. +/// +public class MagenticProgressLedger +{ + internal static readonly BooleanProgressLedgerSlot IsRequestSatisfiedSlot = new("is_request_satisfied", + "Is the request fully satisfied? (True if complete, or False if the original request has yet to be SUCCESSFULLY and FULLY addressed)"); + + internal static readonly BooleanProgressLedgerSlot IsInLoopSlot = new("is_in_loop", + "Are we in a loop where we are repeating the same requests and or getting the same responses as before? " + + "Loops can span multiple turns, and can include repeated actions like scrolling up or down more than a handful of times."); + + internal static readonly BooleanProgressLedgerSlot IsProgressBeingMadeSlot = new("is_progress_being_made", + "Are we making forward progress? (True if just starting, or recent messages are adding value. False if recent " + + "messages show evidence of being stuck in a loop or if there is evidence of significant barriers to success " + + "such as the inability to read from a required file)"); + + internal readonly StringProgressLedgerSlot NextSpeakerSlot; + + internal static readonly StringProgressLedgerSlot InstructionOrQuestionSlot = new("instruction_or_question", + "What instruction or question would you give this team member? (Phrase as if speaking directly to them, and " + + "include any specific information they may need)"); + + internal MagenticProgressLedger(string teamNames, IEnumerable additionalQuestions, JsonElement? state = null) + { + this.NextSpeakerSlot = new("next_speaker", $"Who should speak next? (select from: {teamNames})"); + this.AdditionalQuestions = additionalQuestions as ProgressLedgerSlot[] ?? additionalQuestions.ToArray(); + + if (state != null) + { + this.TryUpdateState(state.Value); + } + } + + internal ProgressLedgerSlot[] AdditionalQuestions { get; } + + internal bool TryUpdateState(JsonElement element) + { + // In principle all of these should be inlineable, but the CodeAnalysis fails to properly chain through the and-chain to realize that + // all must be true for `requiredQuestionsAnswered` to be true, meaning all of the out parameters would be initialized properly. + bool isInLoop = false; + bool isProgressBeingMade = false; + string? nextSpeaker = string.Empty; + string? instructionOrQuestion = string.Empty; + + bool requiredQuestionsAnswered = + IsRequestSatisfiedSlot.TryGetValueFrom(element, out bool isRequestSatisfied) && + IsInLoopSlot.TryGetValueFrom(element, out isInLoop) && + IsProgressBeingMadeSlot.TryGetValueFrom(element, out isProgressBeingMade) && + this.NextSpeakerSlot.TryGetValueFrom(element, out nextSpeaker) && + InstructionOrQuestionSlot.TryGetValueFrom(element, out instructionOrQuestion); + + if (requiredQuestionsAnswered) + { + this.State = element; + + this.IsRequestSatisfied = isRequestSatisfied; + this.IsInLoop = isInLoop; + this.IsProgressBeingMade = isProgressBeingMade; + + this.NextSpeaker = nextSpeaker!; + this.InstructionOrQuestion = instructionOrQuestion!; + } + + // TODO: To what extent do we want to enforce that the additional questions are also answered? + + return requiredQuestionsAnswered; + } + + [JsonInclude] + internal JsonElement? State; + + /// + /// Specifies whether plan execution has started. + /// + [JsonIgnore] + public bool IsStarted => this.State != null; + + /// + /// Specifies whether the task has been fully satisfied. + /// + [JsonIgnore] + public bool IsRequestSatisfied { get; private set; } + + /// + /// Specifies whether the team is in a loop. + /// + [JsonIgnore] + public bool IsInLoop { get; private set; } + + /// + /// Specifies whether the team is making progress on the task. + /// + [JsonIgnore] + public bool IsProgressBeingMade { get; private set; } + + /// + /// Gets the next team member to take a turn. + /// + [JsonIgnore] + public string NextSpeaker { get; private set; } = string.Empty; + + /// + /// Gets the instruction or question to send to the next team member. + /// + [JsonIgnore] + public string InstructionOrQuestion { get; private set; } = string.Empty; + + [JsonIgnore] + internal IEnumerable Slots => + [ + IsRequestSatisfiedSlot, + IsInLoopSlot, + IsProgressBeingMadeSlot, + this.NextSpeakerSlot, + InstructionOrQuestionSlot, + .. this.AdditionalQuestions + ]; + + internal bool TryGetCurrentSlotValue(ProgressLedgerSlot slot, [NotNullWhen(true)] out T? value) + { + if (!this.State.HasValue) + { + value = default; + return false; + } + + return slot.TryGetValueFrom(this.State.Value, out value); + } + + private (string QuestionBlock, string AnswerSchema)? _questionFormatCache; + internal (string QuestionBlock, string AnswerSchema) FormatQuestions() + { + if (!this._questionFormatCache.HasValue) + { + StringBuilder questionBuilder = new(), schemaBuilder = new(); + + schemaBuilder.AppendLine("{"); + foreach (ProgressLedgerSlot slot in this.Slots) + { + questionBuilder.AppendLine(slot.FormattedQuestion); + + schemaBuilder.AppendLine($"\"{slot.Key}\": {{") + .AppendLine($" \"{ProgressLedgerSlot.ValueKey}\": {slot.SchemaType}{slot.SuffixString},") + .AppendLine($" \"{ProgressLedgerSlot.ReasonKey}\": string") + .AppendLine("}"); + } + schemaBuilder.AppendLine("}"); + + this._questionFormatCache = (questionBuilder.ToString(), schemaBuilder.ToString()); + } + + return this._questionFormatCache.Value; + } +} + +internal abstract record ProgressLedgerSlot(string Key, string Question, string? SchemaTypeSuffix = null) +{ + public const string ValueKey = "answer"; + public const string ReasonKey = "reason"; + + internal string SuffixString => this.SchemaTypeSuffix == null ? string.Empty : $"({this.SchemaTypeSuffix})"; + + protected internal abstract string SchemaType { get; } + + public string FormattedQuestion + { + get + { + if (field == null) + { + IEnumerable questionLines = this.Question.Split(['\r', '\n'], StringSplitOptions.RemoveEmptyEntries) + .Select(line => line.TrimEnd()); + + field = $" - {string.Join("\n ", questionLines)}"; + } + + return field; + } + } +} + +internal abstract record ProgressLedgerSlot(string Key, string Question, string? SchemaTypeSuffix = null, JsonSerializerOptions? SerializerOptions = null) + : ProgressLedgerSlot(Key, Question, SchemaTypeSuffix) +{ + protected internal virtual JsonTypeInfo GetJsonTypeInfo() => + ((this.SerializerOptions ?? WorkflowsJsonUtilities.DefaultOptions).TryGetTypeInfo(typeof(T), out JsonTypeInfo? typeInfo) + ? typeInfo as JsonTypeInfo : null) + ?? throw new InvalidOperationException($"Cannot get TypeInfo for {typeof(T)} from {(this.SerializerOptions == null ? "provided" : "default")} SerializationOptions."); + + public bool TryGetValueFrom(JsonElement answers, [NotNullWhen(true)] out T? value) + { + if (answers.TryGetProperty(this.Key, out JsonElement slotElement) && + slotElement.ValueKind != JsonValueKind.Null && + slotElement.TryGetProperty(ValueKey, out JsonElement answerValue)) + { + try + { + T? result = answerValue.Deserialize(this.GetJsonTypeInfo()); + if (result != null) + { + value = result; + return true; + } + } + catch + { + } + } + + value = default; + return false; + } + + public bool TryGetReasonFrom(JsonElement answers, [NotNullWhen(true)] out string? value) + { + if (answers.TryGetProperty(this.Key, out JsonElement slotElement) && + slotElement.ValueKind != JsonValueKind.Null && + slotElement.TryGetProperty(ReasonKey, out JsonElement reasonValue)) + { + try + { + string? result = reasonValue.Deserialize(WorkflowsJsonUtilities.JsonContext.Default.String); + if (result != null) + { + value = result; + return true; + } + } + catch + { + } + } + + value = default; + return false; + } +} + +internal sealed record BooleanProgressLedgerSlot(string Key, string Question, string? SchemaTypeSuffix = null) : ProgressLedgerSlot(Key, Question, SchemaTypeSuffix) +{ + // Since we know the type statically, we can directly return the JsonTypeInfo for string from our JsonContext, + // which is more efficient than looking it up via the options. + protected internal override JsonTypeInfo GetJsonTypeInfo() => WorkflowsJsonUtilities.JsonContext.Default.Boolean; + + protected internal override string SchemaType => "boolean"; +} + +internal sealed record StringProgressLedgerSlot(string Key, string Question, string? SchemaTypeSuffix = null) : ProgressLedgerSlot(Key, Question, SchemaTypeSuffix) +{ + // Since we know the type statically, we can directly return the JsonTypeInfo for string from our JsonContext, + // which is more efficient than looking it up via the options. + protected internal override JsonTypeInfo GetJsonTypeInfo() => WorkflowsJsonUtilities.JsonContext.Default.String; + + protected internal override string SchemaType => "string"; +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/MagenticWorkflowBuilder.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/MagenticWorkflowBuilder.cs new file mode 100644 index 0000000000..f5b8091dc2 --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/MagenticWorkflowBuilder.cs @@ -0,0 +1,158 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.Collections.Generic; +using System.Threading.Tasks; +using Microsoft.Agents.AI.Workflows.Specialized.Magentic; + +using ExecutorFactoryFunc = System.Func, + string, + System.Threading.Tasks.ValueTask>; + +namespace Microsoft.Agents.AI.Workflows; + +/// +/// Fluent builder for creating Magentic One multi-agent orchestration workflows. +/// +/// Magentic One workflows use an LLM-powered manager to coordinate multiple agents through dynamic task planning, progress tracking, +/// and adaptive replanning.The manager creates plans, selects agents, monitors progress, and determines when to replan or complete. +/// +/// The builder provides a fluent API for configuring participants, the manager, optional plan review, checkpointing, and event +/// callbacks. +/// +/// Human-in-the-loop Support: Magentic provides specialized HITL mechanisms via: +/// - `RequirePlanSignoff` - Review and approve/revise plans before execution +/// - Tool approval via `function_approval_request`: Approve individual tool calls on participating agents. Note that tool calls are +/// not supported on the ManagerAgent. +/// +/// +public class MagenticWorkflowBuilder(AIAgent managerAgent) +{ + private readonly List _team = new(); + private string? _name; + private string? _description; + private int _maxStalls = TaskLimits.DefaultMaxStallCount; + private int? _maxRounds; + private int? _maxResets; + private bool _requirePlanSignoff = true; + + /// + public MagenticWorkflowBuilder AddParticipants(params IEnumerable agents) + { + this._team.AddRange(agents); + return this; + } + + /// + public MagenticWorkflowBuilder WithName(string name) + { + this._name = name; + return this; + } + + /// + public MagenticWorkflowBuilder WithDescription(string description) + { + this._description = description; + return this; + } + + /// + /// Set the maximum number of coordination rounds. means unlimited. + /// + /// + public MagenticWorkflowBuilder WithMaxRounds(int? maxRounds = null) + { + this._maxRounds = maxRounds; + return this; + } + + /// + /// Set the maximum number ofnumber of resets allowed. means unlimited. + /// + /// + public MagenticWorkflowBuilder WithMaxResets(int? maxResets = null) + { + this._maxResets = maxResets; + return this; + } + + /// + /// Set the maximum number of consecutive rounds without progress before replan (default 3). + /// + /// + public MagenticWorkflowBuilder WithMaxStalls(int maxStalls = TaskLimits.DefaultMaxStallCount) + { + this._maxStalls = maxStalls; + return this; + } + + /// + /// If , requires human approval of the initial plan or any updates before proceeding. True by default. + /// + /// + /// + public MagenticWorkflowBuilder RequirePlanSignoff(bool requirePlanSignoff = true) + { + this._requirePlanSignoff = requirePlanSignoff; + return this; + } + + private WorkflowBuilder ReduceToWorkflowBuilder() + { + // Create a copy of the team so that improper modifications by using the builder after .Build() do not affect the + // workflow in unexpected ways. + List team = [.. this._team]; + + ExecutorBinding orchestrator = CreateOrchestratorBinding(managerAgent, team, this.Limits, this._requirePlanSignoff); + WorkflowBuilder result = new(orchestrator); + + AIAgentHostOptions options = new() + { + ReassignOtherAgentsAsUsers = true, + ForwardIncomingMessages = false + }; + + List teamBindings = []; + foreach (AIAgent agent in team) + { + ExecutorBinding binding = agent.BindAsExecutor(options); + teamBindings.Add(binding); + + result.AddEdge(binding, orchestrator); + } + + result.AddFanOutEdge(orchestrator, teamBindings) + .WithOutputFrom(orchestrator); + + if (!string.IsNullOrWhiteSpace(this._name)) + { + result.WithName(this._name); + } + + if (!string.IsNullOrWhiteSpace(this._description)) + { + result.WithDescription(this._description); + } + + return result; + } + + /// + public Workflow Build() => this.ReduceToWorkflowBuilder().Build(); + + private TaskLimits Limits => new( + MaxRoundCount: this._maxRounds, + MaxResetCount: this._maxResets, + MaxStallCount: this._maxStalls); + + private static ExecutorBinding CreateOrchestratorBinding(AIAgent managerAgent, List team, TaskLimits limits, bool requirePlanSignoff) + { + ExecutorFactoryFunc factory = CreateOrchestratorAsync; + return factory.BindExecutor(nameof(MagenticOrchestrator)); + + ValueTask CreateOrchestratorAsync(ExecutorConfig options, string sessionId) + { + return new(new MagenticOrchestrator(managerAgent, team, limits, requirePlanSignoff)); + } + } +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/ResetChatSignal.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/ResetChatSignal.cs new file mode 100644 index 0000000000..c8013ded8f --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/ResetChatSignal.cs @@ -0,0 +1,9 @@ +// Copyright (c) Microsoft. All rights reserved. + +namespace Microsoft.Agents.AI.Workflows; + +/// +/// Notifies an AIAgent-hosting executor that it should reset its conversation state, and start a new session, if appropriate. +/// Note that for Agent Orchestrations, only Magentic makes use of this functionality. +/// +public sealed record ResetChatSignal(); diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/AIAgentHostExecutor.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/AIAgentHostExecutor.cs index 885dbc4f57..cd20fc4336 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/AIAgentHostExecutor.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/AIAgentHostExecutor.cs @@ -24,7 +24,7 @@ internal static class TurnExtensions => handoffState.TurnToken.ShouldEmitStreamingEvents(agentSetting); } -internal sealed class AIAgentHostExecutor : ChatProtocolExecutor +internal class AIAgentHostExecutor : ChatProtocolExecutor { private readonly AIAgent _agent; private readonly AIAgentHostOptions _options; @@ -40,7 +40,9 @@ internal sealed class AIAgentHostExecutor : ChatProtocolExecutor StringMessageChatRole = ChatRole.User }; - public AIAgentHostExecutor(AIAgent agent, AIAgentHostOptions options) : base(id: agent.GetDescriptiveId(), + public static string IdFor(AIAgent agent) => agent.GetDescriptiveId(); + + public AIAgentHostExecutor(AIAgent agent, AIAgentHostOptions options) : base(id: IdFor(agent), s_defaultChatProtocolOptions, declareCrossRunShareable: false) // Explicitly false, because we maintain turn state on the instance { @@ -67,7 +69,14 @@ internal sealed class AIAgentHostExecutor : ChatProtocolExecutor protected override ProtocolBuilder ConfigureProtocol(ProtocolBuilder protocolBuilder) { - return this.ConfigureUserInputHandling(base.ConfigureProtocol(protocolBuilder)); + return this.ConfigureUserInputHandling(base.ConfigureProtocol(protocolBuilder)) + .ConfigureRoutes(routeBuilder => routeBuilder.AddHandler(this.ResetChat)); + } + + internal void ResetChat(ResetChatSignal signal, IWorkflowContext context) + { + this._session = null; + this._currentTurnEmitEvents = null; } private ValueTask HandleUserInputResponseAsync( diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/ChatMessageExtensions.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/ChatMessageExtensions.cs new file mode 100644 index 0000000000..6230711077 --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/ChatMessageExtensions.cs @@ -0,0 +1,175 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using System.Text; +using System.Text.Json; +using System.Text.RegularExpressions; +using Microsoft.Extensions.AI; + +namespace Microsoft.Agents.AI.Workflows.Specialized.Magentic; + +internal static partial class ChatMessageExtensions +{ + private static void ProcessAIContents(StringBuilder resultBuilder, IEnumerable contents, StreamingToolCallResultPairMatcher? pairMatcher = null) + { + pairMatcher ??= new(); + + foreach (AIContent content in contents) + { + switch (content) + { + case TextContent textContent: + resultBuilder.AppendLine(textContent.Text); + break; + + //case DataContent dataContent: + // // We really do not know how to deal with anything other than image data with descriptions, which is not + // // a well-defined concept in MEAI (as contrasted with AutoGen's ImageContent type) + // break; + + case ErrorContent errorContent: + resultBuilder.AppendLine($"[ERROR{(errorContent.ErrorCode != null ? $"(Code={errorContent.ErrorCode})" : string.Empty)}]"); + resultBuilder.AppendLine(errorContent.Message); + + if (errorContent.Details != null) + { + resultBuilder.Append("Details:").AppendLine(errorContent.Details); + } + + break; + + case FunctionCallContent functionCallContent: + pairMatcher.CollectFunctionCall(functionCallContent); + break; + + case FunctionResultContent functionResultContent: + pairMatcher.TryResolveFunctionCall(functionResultContent, out string? functionName); + string result = functionResultContent.Result?.ToString() ?? string.Empty; + + resultBuilder.AppendLine($"[Tool Call '{functionName ?? functionResultContent.CallId}' Result]") + .AppendLine(result); + + break; + + case McpServerToolCallContent mstContent: + pairMatcher.CollectMcpServerToolCall(mstContent); + break; + + case McpServerToolResultContent mstResultContent: + if (mstResultContent.Outputs?.Any() is true) + { + pairMatcher.TryResolveMcpServerToolCall(mstResultContent, out string? mcpServerToolName); + resultBuilder.AppendLine($"[Start MCP Server Tool Call '{mcpServerToolName ?? mstResultContent.CallId}' Results]"); + + ProcessAIContents(resultBuilder, mstResultContent.Outputs!); + + resultBuilder.AppendLine($"[End MCP Server Tool Call '{mcpServerToolName ?? mstResultContent.CallId}']"); + } + + break; + case TextReasoningContent reasoningContent: + if (!string.IsNullOrWhiteSpace(reasoningContent.Text)) + { + resultBuilder.Append("[Reasoning] ") + .AppendLine(reasoningContent.Text); + } + + break; + + case UriContent uriContent: + resultBuilder.AppendLine(uriContent.Uri.ToString()); + break; + } + } + } + + public static string GetText(this List messages) + { + if (messages.Count == 0) + { + return string.Empty; + } + + StringBuilder builder = new(); + StreamingToolCallResultPairMatcher pairMatcher = new(); + foreach (ChatMessage message in messages) + { + ProcessAIContents(builder, message.Contents, pairMatcher); + } + + return builder.ToString(); + } + + private const string FencedJsonRegexPattern = @"```(?[a-z]+)?\s*(?\{[\s\S]*?\})\s*```"; +#if NET + [GeneratedRegex(FencedJsonRegexPattern, RegexOptions.IgnoreCase | RegexOptions.CultureInvariant | RegexOptions.ExplicitCapture)] + public static partial Regex FencedJsonRegex(); +#else + public static Regex FencedJsonRegex() => s_fencedJsonRegex; + private static readonly Regex s_fencedJsonRegex = + new(FencedJsonRegexPattern, RegexOptions.Compiled | RegexOptions.CultureInvariant | RegexOptions.ExplicitCapture); +#endif + + internal static JsonElement ExtractJson(string messageText) + { + Match match = FencedJsonRegex().Match(messageText); + if (match.Success) + { + return JsonElement.Parse(match.Groups["json"].Value); + } + + int start = messageText.IndexOf('{'), scanHead = start; + int? end = null; + + if (scanHead < 0) + { + throw new InvalidOperationException("No JSON object found."); + } + + int depth = 0; + bool inQuotes = false, inEscape = false; + for (; scanHead < messageText.Length && end is null; scanHead++) + { + if (inEscape) + { + inEscape = false; + continue; + } + + switch (messageText[scanHead]) + { + case '{' when !inQuotes: + depth++; + break; + case '}' when !inQuotes: + depth--; + if (depth == 0) + { + end = scanHead; + } + + break; + case '\"': + // We already handled inEscape, so we can always flip inQuotes here + inQuotes = !inQuotes; + break; + case '\\': + Debug.Assert(!inEscape); + inEscape = true; + break; + } + } + + if (end is null) + { + throw new InvalidOperationException("Unbalanced JSON braces."); + } + + return JsonElement.Parse(messageText.Substring(start, end.Value - start + 1)); + } + + public static JsonElement ExtractJson(this ChatMessage message) => ExtractJson(message.Text); +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/ExecutorAgentHarness.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/ExecutorAgentHarness.cs new file mode 100644 index 0000000000..858e91bd3e --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/ExecutorAgentHarness.cs @@ -0,0 +1,72 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.Collections.Generic; +using System.Text.Json; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.AI; + +namespace Microsoft.Agents.AI.Workflows.Specialized.Magentic; + +internal sealed class ExecutorAgentHarness(AIAgent agent, AIAgentUnservicedRequestsCollector collector) +{ + internal const string AgentSessionKey = nameof(AgentSession); + private AgentSession? _session; + + private async ValueTask EnsureSessionAsync(IWorkflowContext context, CancellationToken cancellationToken) => + this._session ??= await agent.CreateSessionAsync(cancellationToken).ConfigureAwait(false); + + public async ValueTask InvokeAgentAsync(IEnumerable messages, IWorkflowContext context, bool emitUpdateEvents, CancellationToken cancellationToken = default) + { + AgentResponse response; + + if (emitUpdateEvents) + { + // Run the agent in streaming mode only when agent run update events are to be emitted. + IAsyncEnumerable agentStream = agent.RunStreamingAsync( + messages, + await this.EnsureSessionAsync(context, cancellationToken).ConfigureAwait(false), + cancellationToken: cancellationToken); + + List updates = []; + await foreach (AgentResponseUpdate update in agentStream.ConfigureAwait(false)) + { + await context.YieldOutputAsync(update, cancellationToken).ConfigureAwait(false); + collector.ProcessAgentResponseUpdate(update); + updates.Add(update); + } + + response = updates.ToAgentResponse(); + } + else + { + // Otherwise, run the agent in non-streaming mode. + response = await agent.RunAsync(messages, + await this.EnsureSessionAsync(context, cancellationToken).ConfigureAwait(false), + cancellationToken: cancellationToken) + .ConfigureAwait(false); + + collector.ProcessAgentResponse(response); + } + + return response; + } + + public async ValueTask SerializeSessionAsync(CancellationToken cancellationToken) + => this._session == null + ? null + : await agent.SerializeSessionAsync(this._session, cancellationToken: cancellationToken).ConfigureAwait(false); + + public async ValueTask DeserializeSessionAsync(JsonElement? serializedSession, CancellationToken cancellationToken) + { + this._session = serializedSession == null + ? null + : await agent.DeserializeSessionAsync(serializedSession.Value, cancellationToken: cancellationToken) + .ConfigureAwait(false); + } + + public void ResetSession() + { + this._session = null; + } +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticConstants.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticConstants.cs new file mode 100644 index 0000000000..2ff41cc43d --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticConstants.cs @@ -0,0 +1,8 @@ +// Copyright (c) Microsoft. All rights reserved. + +namespace Microsoft.Agents.AI.Workflows.Specialized.Magentic; + +internal static class MagenticConstants +{ + public const string MagenticTaskContextKey = nameof(MagenticTaskContextKey); +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticManager.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticManager.cs new file mode 100644 index 0000000000..936d9d951e --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticManager.cs @@ -0,0 +1,122 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.Runtime.ExceptionServices; +using System.Text.Json; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.AI; + +namespace Microsoft.Agents.AI.Workflows.Specialized.Magentic; + +internal class MagenticManager(AIAgent managerAgent) +{ + private static async ValueTask CheckResponseAsync(Task responseTask, IWorkflowContext context, CancellationToken cancellationToken) + { + AgentResponse response = await responseTask.ConfigureAwait(false); + + if (response.Messages.Count == 0) + { + throw new InvalidOperationException("Planner Agent did not return any messages."); + } + + if (response.Messages.Count > 1) + { + await context.AddEventAsync(new WorkflowWarningEvent("Planner Agent returned multiple messages; using the last one."), cancellationToken) + .ConfigureAwait(false); + } + + return response.Messages[response.Messages.Count - 1]; + } + + private ValueTask InvokeAgentAsync(IEnumerable messages, IWorkflowContext context, CancellationToken cancellationToken, AgentSession? session = null) + => CheckResponseAsync(managerAgent.RunAsync(messages, session, cancellationToken: cancellationToken), context, cancellationToken); + + public async ValueTask UpdatePlanAsync(MagenticTaskContext taskContext, IWorkflowContext context, CancellationToken cancellationToken) + { + // If we already have a TaskLedger, we need to update the facts based on the existing factset; otherwise, we use the initial facts construction + bool isReplan = taskContext.TaskLedger != null; + + AgentSession localSession = await managerAgent.CreateSessionAsync(cancellationToken).ConfigureAwait(false); + + ChatMessage factsRequest = new(ChatRole.User, isReplan ? taskContext.ToTaskLedgerFactsUpdatePrompt() : taskContext.ToTaskLedgerFactsPrompt()); + ChatMessage updatedFacts = await this.InvokeAgentAsync( + messages: [.. taskContext.ChatHistory, factsRequest], + context, + cancellationToken, + localSession) + .ConfigureAwait(false); + + ChatMessage planRequest = new(ChatRole.User, isReplan ? taskContext.ToTaskLedgerPlanUpdatePrompt() : taskContext.ToTaskLedgerPlanPrompt()); + ChatMessage updatedPlan = await this.InvokeAgentAsync( + // We rely on the AgentSession to maintain the context of the conversation, so we don't include the + // history, facts request, or updated facts in the messages list. + messages: [planRequest], + context, + cancellationToken, + localSession) + .ConfigureAwait(false); + + taskContext.ChatHistory.AddRange([factsRequest, updatedFacts, planRequest, updatedPlan]); + + return new(updatedFacts, updatedPlan); + } + + public async ValueTask UpdateProgressLedgerAsync(MagenticTaskContext taskContext, IWorkflowContext context, CancellationToken cancellationToken) + { + ChatMessage progressRequest = new(ChatRole.User, taskContext.ToProgressLedgerPrompt()); + + ExceptionDispatchInfo? lastException = null; + int maxRetryCount = taskContext.TaskLimits.MaxProgressLedgerRetryCount; + for (int attempts = 0; attempts < maxRetryCount; attempts++) + { + ChatMessage progressUpdateMessage = await this.InvokeAgentAsync( + messages: [.. taskContext.ChatHistory, progressRequest], + context, + cancellationToken) + .ConfigureAwait(false); + + try + { + lastException = null; + JsonElement stateUpdateJson = progressUpdateMessage.ExtractJson(); + if (!taskContext.ProgressLedger.TryUpdateState(stateUpdateJson)) + { + throw new InvalidOperationException("Could not answer progress ledger questions with provided JSON."); + } + + break; + } + catch (Exception e) + { + lastException = ExceptionDispatchInfo.Capture(e); + + string warnString = $"Progress ledger JSON parse failed (attempt {attempts}/{maxRetryCount}): {e}"; + await context.AddEventAsync(new WorkflowWarningEvent(warnString), cancellationToken).ConfigureAwait(false); + + if (attempts < maxRetryCount) + { + await Task.Delay(250 * attempts, cancellationToken).ConfigureAwait(false); + } + } + } + + lastException?.Throw(); + } + + public async ValueTask PrepareFinalAnswerAsync(MagenticTaskContext taskContext, IWorkflowContext context, CancellationToken cancellationToken) + { + ChatMessage finalAnswerRequest = new(ChatRole.User, taskContext.ToFinalAnswerPrompt()); + ChatMessage finalAnswer = await this.InvokeAgentAsync([.. taskContext.ChatHistory, finalAnswerRequest], context, cancellationToken) + .ConfigureAwait(false); + + return new(ChatRole.Assistant, finalAnswer.Text) + { + AuthorName = finalAnswer.AuthorName ?? nameof(MagenticManager), + MessageId = finalAnswer.MessageId ?? Guid.NewGuid().ToString("N"), + CreatedAt = finalAnswer.CreatedAt ?? DateTimeOffset.UtcNow, + RawRepresentation = finalAnswer.RawRepresentation, + }; + } +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticOrchestrator.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticOrchestrator.cs new file mode 100644 index 0000000000..b272ed102d --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticOrchestrator.cs @@ -0,0 +1,326 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text.Json.Serialization; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.AI; + +namespace Microsoft.Agents.AI.Workflows.Specialized.Magentic; + +/// +/// Base type for Magentic Orchestration Events +/// +/// +[JsonDerivedType(typeof(MagenticPlanCreatedEvent))] +[JsonDerivedType(typeof(MagenticReplannedEvent))] +[JsonDerivedType(typeof(MagenticProgressLedgerUpdatedEvent))] +public abstract class MagenticOrchestratorEvent(object? data) : WorkflowEvent(data) +{ +} + +/// +/// Represents the creation of the initial plan +/// +/// +public sealed class MagenticPlanCreatedEvent(ChatMessage fullTaskLeger) : MagenticOrchestratorEvent(fullTaskLeger) +{ + /// + /// A containing the initial plan. + /// + public ChatMessage FullTaskLedger { get; } = fullTaskLeger; +} + +/// +/// Represents the creation of a new plan in response to a stall. +/// +/// +public sealed class MagenticReplannedEvent(ChatMessage fullTaskLeger) : MagenticOrchestratorEvent(fullTaskLeger) +{ + /// + /// A containing the new plan. + /// + public ChatMessage FullTaskLedger { get; } = fullTaskLeger; +} + +/// +/// Represents an update to the when running a coordination round. +/// +/// +public sealed class MagenticProgressLedgerUpdatedEvent(MagenticProgressLedger progressLedger) : MagenticOrchestratorEvent(progressLedger) +{ + /// + /// The new state of the + /// + public MagenticProgressLedger ProgressLedger { get; } = progressLedger; +} + +/// +/// Magentic orchestrator that defines the workflow structure. +/// +/// This orchestrator manages the overall Magentic workflow in the following structure: +/// +/// 1. Upon receiving the task(a list of messages), it creates the plan using the manager then runs the inner loop. +/// 2. The inner loop is distributed and implementation is decentralized. In the orchestrator, it is responsible for: +/// - Creating the progress ledger using the manager. +/// - Checking for task completion. +/// - Detecting stalling or looping and triggering replanning if needed. +/// - Sending requests to participants based on the progress ledger's next speaker. +/// - Issue requests for human intervention if enabled and needed. +/// 3. The inner loop waits for responses from the selected participant, then continues the loop. +/// 4. The orchestrator breaks out of the inner loop when the replanning or final answer conditions are met. +/// 5. The outer loop handles replanning and reenters the inner loop. +/// +/// +/// +/// +/// +internal class MagenticOrchestrator(AIAgent managerAgent, List team, TaskLimits limits, bool requirePlanSignoff) + : ChatProtocolExecutor(nameof(MagenticOrchestrator), s_options, declareCrossRunShareable: false) +{ + private readonly MagenticManager _manager = new(managerAgent); + + private static readonly ChatProtocolExecutorOptions s_options = new() + { + StringMessageChatRole = ChatRole.User, + AutoSendTurnToken = false + }; + + private MagenticTaskContext? _taskContext; + private PortBinding? _planReviewPort; + + protected override ProtocolBuilder ConfigureProtocol(ProtocolBuilder protocolBuilder) + { + return base.ConfigureProtocol(protocolBuilder).ConfigureRoutes(ConfigureRoutes); + + void ConfigureRoutes(RouteBuilder routeBuilder) => routeBuilder.AddPortHandler( + "RequestPlanReview", + this.ProcessPlanReviewAsync, + out this._planReviewPort); + } + + private ValueTask SubmitPlanReviewRequestAsync(MagenticTaskContext taskContext, IWorkflowContext workflowContext) + { + MagenticProgressLedger? progressLedger = taskContext.ProgressLedger; + if (progressLedger?.IsStarted is not true) + { + progressLedger = null; + } + + MagenticPlanReviewRequest request = new(taskContext.TaskLedger!.CurrentPlan, progressLedger, taskContext.IsStalled); + + return this._planReviewPort!.PostRequestAsync(request); + } + + private async ValueTask ProcessPlanReviewAsync(MagenticPlanReviewResponse response, IWorkflowContext context, CancellationToken cancellationToken) + { + /* + Handle the human response to the plan review request. + + Logic: + There are code paths which will trigger a plan review request to the human: + - Initial plan creation if `require_plan_signoff` is True. + - Potentially during the inner loop if stalling is detected (resetting and replanning). + + The human can either approve the plan or request revisions with comments. + - If approved, proceed to run the outer loop, which simply adds the task ledger + to the conversation and enters the inner loop. + - If revision requested, append the review comments to the chat history, + trigger replanning via the manager, emit a REPLANNED event, then run the outer loop. + + */ + if (this._taskContext == null || this._taskContext.TaskLedger == null) + { + throw new InvalidOperationException("Magentic Orchestration was not initialized correctly."); + } + + if (this._taskContext.IsTerminated) + { + throw new InvalidOperationException("Magentic Orchestration has already been terminated and cannot process new messages. Please start a new session."); + } + + if (response.IsApproved) + { + await this.DelegateToTeamAsync(this._taskContext, context, cancellationToken).ConfigureAwait(false); + } + else + { + this._taskContext.ChatHistory.AddRange(response.Review); + + await this.UpdatePlanAndDelegateAsync(this._taskContext, context, cancellationToken).ConfigureAwait(false); + } + } + + private async ValueTask UpdatePlanAndDelegateAsync(MagenticTaskContext taskContext, IWorkflowContext context, CancellationToken cancellationToken) + { + bool isReplan = taskContext.TaskLedger != null; + + taskContext.TaskLedger = await this._manager.UpdatePlanAsync(taskContext, context, cancellationToken) + .ConfigureAwait(false); + + this._fullTaskLedgerMessage = new(ChatRole.User, taskContext.ToTaskLedgerFullPrompt()); + taskContext.ChatHistory.Add(this._fullTaskLedgerMessage); + + await context.AddEventAsync(isReplan + ? new MagenticReplannedEvent(this._fullTaskLedgerMessage) + : new MagenticPlanCreatedEvent(this._fullTaskLedgerMessage), cancellationToken).ConfigureAwait(false); + + if (requirePlanSignoff) + { + await this.SubmitPlanReviewRequestAsync(taskContext, context).ConfigureAwait(false); + } + else + { + await this.DelegateToTeamAsync(taskContext, context, cancellationToken).ConfigureAwait(false); + } + } + + protected override async ValueTask TakeTurnAsync(List messages, IWorkflowContext context, bool? emitEvents, CancellationToken cancellationToken = default) + { + // First Turn: Initialize the task context and send the initial messages to the planner agent + this._taskContext ??= new(messages, team, limits, emitEvents, []); + await this.UpdatePlanAndDelegateAsync(this._taskContext, context, cancellationToken).ConfigureAwait(false); + } + + private ChatMessage? _fullTaskLedgerMessage; + private ValueTask DelegateToTeamAsync(MagenticTaskContext taskContext, IWorkflowContext context, CancellationToken cancellationToken) + { + return this.RunCoordinationRoundAsync(taskContext, context, cancellationToken); + } + + private async ValueTask RunCoordinationRoundAsync(MagenticTaskContext taskContext, IWorkflowContext context, CancellationToken cancellationToken) + { + (bool hitRoundLimit, bool hitResetLimit) = taskContext.CheckLimits(); + + if (hitRoundLimit || hitResetLimit) + { + string limitType = hitRoundLimit ? "round" : "reset"; + + List messages = [new(ChatRole.Assistant, $"Task execution stopped due to hitting the maximum {limitType} count limit.")]; + await context.YieldOutputAsync(messages, cancellationToken).ConfigureAwait(false); + taskContext.IsTerminated = true; + + return; + } + + taskContext.TaskCounters.RoundCount++; + + // Update the Progress Ledger + try + { + await this._manager.UpdateProgressLedgerAsync(taskContext, context, cancellationToken).ConfigureAwait(false); + + await context.AddEventAsync(new MagenticProgressLedgerUpdatedEvent(taskContext.ProgressLedger), cancellationToken) + .ConfigureAwait(false); + } + // Retry on exception to max retry count, unless it is OperationCancelledException - in that case exit the loop right away + catch (Exception ex) when (ex is not OperationCanceledException) + { + await context.AddEventAsync(new WorkflowWarningEvent($"Magentic Orchestrator: Progress ledger creation failed, triggering reset: {ex}"), cancellationToken) + .ConfigureAwait(false); + + await this.ResetAndReplanAsync(taskContext, context, cancellationToken).ConfigureAwait(false); + return; + } + + // Check and handle finish condition + if (taskContext.ProgressLedger.IsRequestSatisfied) + { + await this.PrepareFinalAnswerAsync(taskContext, context, cancellationToken).ConfigureAwait(false); + return; + } + + // Check and handle stalls + if (taskContext.ProgressLedger.IsInLoop || !taskContext.ProgressLedger.IsProgressBeingMade) + { + taskContext.TaskCounters.StallCount++; + } + else + { + taskContext.TaskCounters.StallCount = Math.Max(0, taskContext.TaskCounters.StallCount - 1); + } + + if (taskContext.IsStalled) + { + await this.ResetAndReplanAsync(taskContext, context, cancellationToken).ConfigureAwait(false); + return; + } + + // Prepare to delegate to the next speaker + string nextSpeaker = taskContext.ProgressLedger.NextSpeaker; + if (string.IsNullOrEmpty(nextSpeaker)) + { + await context.AddEventAsync(new WorkflowWarningEvent("Next speaker answer empty; selecting first participant as fallback"), cancellationToken) + .ConfigureAwait(false); + nextSpeaker = team.First().Name!; + } + + AIAgent? nextAgent = team.FirstOrDefault(agent => agent.Name == nextSpeaker); + if (nextAgent == null) + { + await context.AddEventAsync(new WorkflowWarningEvent($"Invalid next speaker: {nextSpeaker}"), cancellationToken) + .ConfigureAwait(false); + await this.PrepareFinalAnswerAsync(taskContext, context, cancellationToken).ConfigureAwait(false); + return; + } + + if (!string.IsNullOrWhiteSpace(taskContext.ProgressLedger.InstructionOrQuestion)) + { + ChatMessage instruction = new(ChatRole.Assistant, taskContext.ProgressLedger.InstructionOrQuestion); + taskContext.ChatHistory.Add(instruction); + + await context.SendMessageAsync(instruction, cancellationToken).ConfigureAwait(false); + } + + string nextExecutorId = AIAgentHostExecutor.IdFor(nextAgent); + await context.SendMessageAsync(new TurnToken(taskContext.EmitUpdateEvents), nextExecutorId, cancellationToken).ConfigureAwait(false); + } + + private async ValueTask ResetAndReplanAsync(MagenticTaskContext taskContext, IWorkflowContext context, CancellationToken cancellationToken) + { + taskContext.Reset(); + await context.SendMessageAsync(new ResetChatSignal(), cancellationToken: cancellationToken).ConfigureAwait(false); + + await this.UpdatePlanAndDelegateAsync(taskContext, context, cancellationToken).ConfigureAwait(false); + } + + private async ValueTask PrepareFinalAnswerAsync(MagenticTaskContext taskContext, IWorkflowContext context, CancellationToken cancellationToken) + { + List messages = [await this._manager.PrepareFinalAnswerAsync(taskContext, context, cancellationToken).ConfigureAwait(false)]; + await context.YieldOutputAsync(messages, cancellationToken).ConfigureAwait(false); + taskContext.IsTerminated = true; + } + + private const string CurrentTurnEmitUpdateEventsKey = nameof(CurrentTurnEmitUpdateEventsKey); + protected internal override async ValueTask OnCheckpointingAsync(IWorkflowContext context, CancellationToken cancellationToken = default) + { + Task contextStateTask = this._taskContext == null + ? Task.CompletedTask + : context.QueueStateUpdateAsync(MagenticConstants.MagenticTaskContextKey, + this._taskContext.ExportState(), + cancellationToken: cancellationToken) + .AsTask(); + + await Task.WhenAll(base.OnCheckpointingAsync(context, cancellationToken).AsTask(), + contextStateTask).ConfigureAwait(false); + } + + protected internal override async ValueTask OnCheckpointRestoredAsync(IWorkflowContext context, CancellationToken cancellationToken = default) + { + await Task.WhenAll(base.OnCheckpointRestoredAsync(context, cancellationToken).AsTask(), LoadContextStateAsync()) + .ConfigureAwait(false); + + async Task LoadContextStateAsync() + { + MagenticTaskState? state = await context.ReadStateAsync(MagenticConstants.MagenticTaskContextKey, cancellationToken: cancellationToken) + .ConfigureAwait(false); + + if (state != null) + { + this._taskContext = new MagenticTaskContext(state, team, limits, []); + } + } + } +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticTaskContext.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticTaskContext.cs new file mode 100644 index 0000000000..0db289126e --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticTaskContext.cs @@ -0,0 +1,95 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text.Json; +using Microsoft.Extensions.AI; + +namespace Microsoft.Agents.AI.Workflows.Specialized.Magentic; + +internal record TaskLimits(int MaxStallCount = TaskLimits.DefaultMaxStallCount, + int? MaxRoundCount = null, + int? MaxResetCount = null, + int MaxProgressLedgerRetryCount = TaskLimits.DefaultMaxProgressLedgerRetryCount) +{ + public const int DefaultMaxStallCount = 3; + public const int DefaultMaxProgressLedgerRetryCount = 3; +} + +internal record TaskLedger(ChatMessage CurrentFacts, ChatMessage CurrentPlan); + +internal class TaskCounters +{ + public int RoundCount { get; set; } + public int StallCount { get; set; } + public int ResetCount { get; set; } +} + +internal record MagenticTaskState(List TaskDefinition, List ChatHistory, TaskLedger? TaskLedger, JsonElement? ProgressLedgerState, TaskCounters Counters, bool Terminated, bool? EmitUpdateEvents) +{ +} + +internal class MagenticTaskContext(List taskDefinition, List team, TaskLimits limits, bool? emitUpdateEvents, IEnumerable additionalProgressQuestions) +{ + internal MagenticTaskContext(MagenticTaskState state, List team, TaskLimits limits, IEnumerable additionalProgressQuestions) + : this(state.TaskDefinition, team, limits, state.EmitUpdateEvents, additionalProgressQuestions) + { + this.TaskLedger = state.TaskLedger; + this.TaskCounters = state.Counters; + this.ChatHistory = state.ChatHistory; + this.IsTerminated = state.Terminated; + + if (state.ProgressLedgerState.HasValue && !this.ProgressLedger.TryUpdateState(state.ProgressLedgerState.Value)) + { + throw new InvalidOperationException("Could not load progress ledger state value"); + } + } + + public string Task { get; } = taskDefinition.GetText(); + + public string TeamDescription { get; } = GetTeamDescription(team); + + public List ChatHistory { get; internal set; } = new(); + + public TaskLedger? TaskLedger { get; internal set; } + + public TaskLimits TaskLimits => limits; + + public bool IsTerminated { get; internal set; } + + public bool IsStalled => this.TaskCounters.StallCount >= this.TaskLimits.MaxStallCount; + + public (bool HitRoundLimit, bool HitResetLimit) CheckLimits() + { + return (this.TaskLimits.MaxRoundCount.HasValue && this.TaskLimits.MaxRoundCount.Value <= this.TaskCounters.RoundCount, + this.TaskLimits.MaxResetCount.HasValue && this.TaskLimits.MaxResetCount.Value <= this.TaskCounters.ResetCount); + } + + public TaskCounters TaskCounters { get; internal set; } = new(); + + public MagenticProgressLedger ProgressLedger { get; } = new(GetTeamNames(team), additionalProgressQuestions); + public bool? EmitUpdateEvents => emitUpdateEvents; + + public static string GetTeamDescription(IEnumerable team) + { + return string.Join("\n", team.Select(agent => $"- {agent.Name}: {agent.Description}")); + } + + public static string GetTeamNames(IEnumerable team) + { + return string.Join(", ", team.Select(agent => agent.Name)); + } + + public MagenticTaskState ExportState() + { + return new(taskDefinition, this.ChatHistory, this.TaskLedger, this.ProgressLedger.State, this.TaskCounters, this.IsTerminated, this.EmitUpdateEvents); + } + + internal void Reset() + { + this.ChatHistory.Clear(); + this.TaskCounters.ResetCount++; + this.TaskCounters.StallCount = 0; + } +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/PromptTemplates.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/PromptTemplates.cs new file mode 100644 index 0000000000..17176a97d4 --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/PromptTemplates.cs @@ -0,0 +1,151 @@ +// Copyright (c) Microsoft. All rights reserved. + +using Microsoft.Extensions.AI; + +namespace Microsoft.Agents.AI.Workflows.Specialized.Magentic; + +internal static class PromptTemplateExtensions +{ + public static string ToTaskLedgerFactsPrompt(this MagenticTaskContext taskContext) + { + return $""" +Below I will present you a request. + +Before we begin addressing the request, please answer the following pre-survey to the best of your ability. + Keep in mind that you are Ken Jennings-level with trivia, and Mensa-level with puzzles, so there should be + a deep well to draw from. + + Here is the request: + +{taskContext.Task} + + Here is the pre-survey: + + 1. Please list any specific facts or figures that are GIVEN in the request itself.It is possible that + there are none. + 2. Please list any facts that may need to be looked up, and WHERE SPECIFICALLY they might be found. + In some cases, authoritative sources are mentioned in the request itself. + 3. Please list any facts that may need to be derived(e.g., via logical deduction, simulation, or computation) + 4. Please list any facts that are recalled from memory, hunches, well-reasoned guesses, etc. + +When answering this survey, keep in mind that "facts" will typically be specific names, dates, statistics, etc. +Your answer should use headings: + + 1. GIVEN OR VERIFIED FACTS + 2. FACTS TO LOOK UP + 3. FACTS TO DERIVE + 4. EDUCATED GUESSES + +DO NOT include any other headings or sections in your response.DO NOT list next steps or plans until asked to do so. +"""; + } + + public static string ToTaskLedgerFactsUpdatePrompt(this MagenticTaskContext taskContext) + { + return $""" +As a reminder, we are working to solve the following task: + +{taskContext.Task} + +It is clear we are not making as much progress as we would like, but we may have learned something new. +Please rewrite the following fact sheet, updating it to include anything new we have learned that may be helpful. + +Example edits can include (but are not limited to) adding new guesses, moving educated guesses to verified facts +if appropriate, etc. Updates may be made to any section of the fact sheet, and more than one section of the fact +sheet can be edited. This is an especially good time to update educated guesses, so please at least add or update +one educated guess or hunch, and explain your reasoning. + +Here is the old fact sheet: + +{taskContext.TaskLedger?.CurrentFacts ?? new(ChatRole.Assistant, string.Empty)} +"""; + } + + public static string ToTaskLedgerPlanPrompt(this MagenticTaskContext taskContext) + { + return $""" +Fantastic. To address this request we have assembled the following team: + +{taskContext.TeamDescription} + +Based on the team composition, and known and unknown facts, please devise a short bullet-point plan for addressing the +original request. Remember, there is no requirement to involve all team members. A team member's particular expertise +may not be needed for this task. +"""; + } + + public static string ToTaskLedgerPlanUpdatePrompt(this MagenticTaskContext taskContext) + { + return $""" +Please briefly explain what went wrong on this last run +(the root cause of the failure), and then come up with a new plan that takes steps and includes hints to overcome prior +challenges and especially avoids repeating the same mistakes. As before, the new plan should be concise, expressed in +bullet-point form, and consider the following team composition: + +{taskContext.TeamDescription} +"""; + } + + public static string ToTaskLedgerFullPrompt(this MagenticTaskContext taskContext) + { + return $""" +We are working to address the following user request: + +{taskContext.Task} + + +To answer this request we have assembled the following team: + +{taskContext.TeamDescription} + + +Here is an initial fact sheet to consider: + +{taskContext.TaskLedger!.CurrentFacts ?? new(ChatRole.Assistant, string.Empty)} + + +Here is the plan to follow as best as possible: + +{taskContext.TaskLedger!.CurrentPlan} +"""; + } + + public static string ToProgressLedgerPrompt(this MagenticTaskContext taskContext) + { + (string questions, string schema) = taskContext.ProgressLedger.FormatQuestions(); + + return $""" +Recall we are working on the following request: + +{taskContext.Task} + +And we have assembled the following team: + +{taskContext.TeamDescription} + +To make progress on the request, please answer the following questions, including necessary reasoning: + +{questions} + +Please output an answer in pure JSON format according to the following schema. The JSON object must be parsable as-is. +DO NOT OUTPUT ANYTHING OTHER THAN JSON, AND DO NOT DEVIATE FROM THIS SCHEMA: + +{schema} +"""; + } + + public static string ToFinalAnswerPrompt(this MagenticTaskContext taskContext) + { + return $""" +We are working on the following task: +{taskContext.Task} + +We have completed the task. + +The above messages contain the conversation that took place to complete the task. + +Based on the information gathered, provide the final answer to the original request. +The answer should be phrased as if you were speaking to the user. +"""; + } +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/StreamingToolCallResultPairMatcher.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/StreamingToolCallResultPairMatcher.cs new file mode 100644 index 0000000000..bb72b8390a --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/StreamingToolCallResultPairMatcher.cs @@ -0,0 +1,77 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using Microsoft.Extensions.AI; + +namespace Microsoft.Agents.AI.Workflows.Specialized.Magentic; + +internal sealed class StreamingToolCallResultPairMatcher +{ + private enum CallType + { + Function, + McpServerTool + } + + private record CallSummaryKey(CallType Type, string CallId); + + private struct ToolCallSummary(CallType callType, string callId, string name) + { + public CallType CallType => callType; + + public string? CallId => callId; + + public string Name => name; + } + + private readonly Dictionary _callSummaries = new(); + + private void Collect(CallType callType, string callId, string name, string callContentTypeName, string resultContentTypeName) + { + CallSummaryKey key = new(callType, callId); + if (this._callSummaries.ContainsKey(key)) + { + throw new InvalidOperationException($"Duplicate {callContentTypeName} with CallId '{callId}' without corresponding {resultContentTypeName}."); + } + + this._callSummaries[key] = new ToolCallSummary(callType, callId, name); + } + + public void CollectFunctionCall(FunctionCallContent callContent) + { + const string FunctionCallContentTypeName = nameof(FunctionCallContent); + const string FunctionResultContentTypeName = nameof(FunctionResultContent); + + this.Collect(CallType.Function, callContent.CallId, callContent.Name, FunctionCallContentTypeName, FunctionResultContentTypeName); + } + + public void CollectMcpServerToolCall(McpServerToolCallContent callContent) + { + const string McpServerToolCallContentTypeName = nameof(McpServerToolCallContent); + const string McpServerToolResultContentTypeName = nameof(McpServerToolResultContent); + + this.Collect(CallType.McpServerTool, callContent.CallId, callContent.Name, McpServerToolCallContentTypeName, McpServerToolResultContentTypeName); + } + + private bool TryResolve(CallType callType, string callId, [NotNullWhen(true)] out string? name) + { + CallSummaryKey key = new(callType, callId); + + bool hasMatchingCall = this._callSummaries.TryGetValue(key, out ToolCallSummary callSummary); + if (hasMatchingCall) + { + this._callSummaries.Remove(key); + } + + name = hasMatchingCall ? callSummary.Name : null; + return hasMatchingCall; + } + + public bool TryResolveFunctionCall(FunctionResultContent resultContent, [NotNullWhen(true)] out string? name) + => this.TryResolve(CallType.Function, resultContent.CallId, out name); + + public bool TryResolveMcpServerToolCall(McpServerToolResultContent resultContent, [NotNullWhen(true)] out string? name) + => this.TryResolve(CallType.McpServerTool, resultContent.CallId, out name); +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/WorkflowEvent.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/WorkflowEvent.cs index 76b379a611..0095e86000 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows/WorkflowEvent.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/WorkflowEvent.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. using System.Text.Json.Serialization; +using Microsoft.Agents.AI.Workflows.Specialized.Magentic; namespace Microsoft.Agents.AI.Workflows; @@ -14,6 +15,8 @@ namespace Microsoft.Agents.AI.Workflows; [JsonDerivedType(typeof(WorkflowWarningEvent))] [JsonDerivedType(typeof(WorkflowOutputEvent))] [JsonDerivedType(typeof(RequestInfoEvent))] +[JsonDerivedType(typeof(MagenticOrchestratorEvent))] + public class WorkflowEvent(object? data = null) { /// diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/WorkflowsJsonUtilities.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/WorkflowsJsonUtilities.cs index 6978cc1c93..8b3d3e4ce8 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows/WorkflowsJsonUtilities.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/WorkflowsJsonUtilities.cs @@ -7,6 +7,7 @@ using System.Text.Json.Serialization; using Microsoft.Agents.AI.Workflows.Checkpointing; using Microsoft.Agents.AI.Workflows.Execution; using Microsoft.Agents.AI.Workflows.Specialized; +using Microsoft.Agents.AI.Workflows.Specialized.Magentic; using Microsoft.Extensions.AI; namespace Microsoft.Agents.AI.Workflows; @@ -97,6 +98,10 @@ internal static partial class WorkflowsJsonUtilities [JsonSerializable(typeof(AIAgentHostState))] [JsonSerializable(typeof(HandoffSharedState))] [JsonSerializable(typeof(HandoffAgentHostState))] + [JsonSerializable(typeof(MagenticPlanReviewRequest))] + [JsonSerializable(typeof(MagenticPlanReviewResponse))] + [JsonSerializable(typeof(MagenticTaskState))] + [JsonSerializable(typeof(ResetChatSignal))] // Event Types //[JsonSerializable(typeof(WorkflowEvent))] diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/MagenticManagerTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/MagenticManagerTests.cs new file mode 100644 index 0000000000..9014463133 --- /dev/null +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/MagenticManagerTests.cs @@ -0,0 +1,153 @@ +// Copyright (c) Microsoft. All rights reserved. + +//using System.Collections.Generic; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using FluentAssertions; +using Microsoft.Agents.AI.Workflows.Specialized.Magentic; +using Microsoft.Extensions.AI; + +namespace Microsoft.Agents.AI.Workflows.UnitTests; + +public class MagenticManagerTests +{ + private static void CheckMessage(ChatMessage message, string expectedText, bool runPropertySmokeTest = false, bool skipCreatedAt = true) + { + message.Text.Should().Be(expectedText); + + if (runPropertySmokeTest) + { + message.AuthorName.Should().Be(nameof(MagenticOrchestrator)); + + if (!skipCreatedAt) + { + message.CreatedAt.Should().NotBeNull().And.NotBeBefore(DateTimeOffset.UtcNow.AddDays(-1)); + } + + message.Role.Should().Be(ChatRole.Assistant); + message.MessageId.Should().NotBeNull(); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Test_MagenticManager_UpdatePlanAsync(bool hasExistingPlan) + { + TestReplayAgent testAgent = new(name: nameof(MagenticOrchestrator), + messages: + [ + [new(ChatRole.Assistant, "Facts")], + [new(ChatRole.Assistant, "Plan")], + ]); + + TestEchoAgent participant = new(name: "Echo"); + MagenticManager manager = new(testAgent); + + MagenticTaskContext taskContext = new([new(ChatRole.User, "Task")], [participant], new TaskLimits(), null, []); + if (hasExistingPlan) + { + taskContext.TaskLedger = new(new(ChatRole.Assistant, "OldFacts"), new(ChatRole.Assistant, "OldPlan")); + } + + TestRunContext runContext = new(); + IWorkflowContext workflowContext = runContext.BindWorkflowContext(nameof(MagenticOrchestrator)); + + TaskLedger newPlan = await manager.UpdatePlanAsync(taskContext, workflowContext, CancellationToken.None); + CheckMessage(newPlan.CurrentFacts, "Facts"); + CheckMessage(newPlan.CurrentPlan, "Plan"); + + taskContext.ChatHistory.Should().HaveCount(4); + + if (hasExistingPlan) + { + ChatMessage factsRequest = taskContext.ChatHistory[0]; + factsRequest.Text.Should().Contain("OldFacts"); + } + + ChatMessage facts = taskContext.ChatHistory[1]; + facts.Should().Be(newPlan.CurrentFacts); + + ChatMessage plan = taskContext.ChatHistory[3]; + plan.Should().Be(newPlan.CurrentPlan); + } + + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(2)] + [InlineData(3)] + [InlineData(4)] + public async Task Test_MagenticManager_UpdateProgressLedgerAsync(int failures) + { + List> turns = + TestProgressLedgerState.MissingRequired.Take(failures) + .Select>( + state => [new ChatMessage(ChatRole.Assistant, state.ToJsonString())]) + .ToList(); + + turns.Should().HaveCount(failures); + turns.Add([new ChatMessage(ChatRole.Assistant, TestProgressLedgerState.Default.ToJsonString())]); + + TestReplayAgent testAgent = new(name: nameof(MagenticOrchestrator), + messages: turns); + + TestEchoAgent participant = new(name: "Echo"); + MagenticManager manager = new(testAgent); + + MagenticTaskContext taskContext = new([new(ChatRole.User, "Task")], [participant], new TaskLimits(), null, []); + taskContext.TaskLedger = new(new(ChatRole.Assistant, "OldFacts"), new(ChatRole.Assistant, "OldPlan")); + + TestRunContext runContext = new(); + IWorkflowContext workflowContext = runContext.BindWorkflowContext(nameof(MagenticOrchestrator)); + + // Precondition check: ProgressLedger should be not "started" + taskContext.ProgressLedger.IsStarted.Should().BeFalse(); + + Func action = () => manager.UpdateProgressLedgerAsync(taskContext, workflowContext, CancellationToken.None).AsTask(); + + if (failures >= taskContext.TaskLimits.MaxProgressLedgerRetryCount) + { + // We expect to see an exception if the number of failures exceeds the maximum retry count + await action.Should().ThrowAsync(); + taskContext.ProgressLedger.IsStarted.Should().BeFalse(); + } + else + { + await action.Should().NotThrowAsync(); + taskContext.ProgressLedger.IsStarted.Should().BeTrue(); + TestProgressLedgerState.Default.Validate(taskContext.ProgressLedger); + } + + int expectedWarnings = Math.Min(failures, 3); + + runContext.Events.Should().HaveCount(expectedWarnings).And.AllBeOfType(); + } + + [Fact] + public async Task Test_MagenticManager_PrepareFinalAnswerAsync() + { + TestReplayAgent testAgent = new(name: nameof(MagenticOrchestrator), + messages: + [ + [ + new(ChatRole.Assistant, "FinalAnswer") + ], + ]); + + TestEchoAgent participant = new(name: "Echo"); + MagenticManager manager = new(testAgent); + + MagenticTaskContext taskContext = new([new(ChatRole.User, "Task")], [participant], new TaskLimits(), null, []); + + TestRunContext runContext = new(); + IWorkflowContext workflowContext = runContext.BindWorkflowContext(nameof(MagenticOrchestrator)); + + ChatMessage answer = await manager.PrepareFinalAnswerAsync(taskContext, workflowContext, CancellationToken.None); + + CheckMessage(answer, "FinalAnswer", true, false); + } +} diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/MagenticProgressLedgerTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/MagenticProgressLedgerTests.cs new file mode 100644 index 0000000000..fdc66f69d9 --- /dev/null +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/MagenticProgressLedgerTests.cs @@ -0,0 +1,214 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Text.Json; +using FluentAssertions; +using Microsoft.Agents.AI.Workflows.Specialized.Magentic; +using Microsoft.Extensions.AI; + +namespace Microsoft.Agents.AI.Workflows.UnitTests; + +public class MagenticProgressLedgerTests +{ + public record KVPair(string key); + public record AnswerReasonPair(bool answer, string reason); + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void Test_ExtractJson_SucceedsWhenInBlockQuote(bool isTagged) + { + // Arrange + string json = isTagged + ? "```json\n{\"key\": \"value\"}\n```" + : "```{\"key\": \"value\"}```"; + + string embedded = $"Some text before the JSON block.\n{json}\nSome text after the JSON block."; + ChatMessage message = new(ChatRole.Assistant, embedded); + + // Act + JsonElement element = message.ExtractJson(); + + // Assert + KVPair? result = element.Deserialize(); + + result.Should().NotBeNull(); + result.key.Should().Be("value"); + } + + [Fact] + public void Test_ExtractJson_SucceedsWhenScanning() + { + // Arrange + ChatMessage message = new(ChatRole.Assistant, +""" + Some text before the JSON embed. + {"key": "value"} + + Some text after the JSON embed. +"""); + + // Act + JsonElement element = message.ExtractJson(); + + // Assert + KVPair? result = element.Deserialize(); + + result.Should().NotBeNull(); + result.key.Should().Be("value"); + } + + [Fact] + public void Test_ExtractJson_FailsWhenUnbalanced() + { + // Arrange + ChatMessage message = new(ChatRole.Assistant, +""" + Some text before the JSON embed. + {"key": { "key2": "value" } + + Some text after the JSON embed. +"""); + + // Act + Func action = () => message.ExtractJson(); + + // Assert + action.Should().Throw(); + } + + [Fact] + public void Test_ExtractJson_FailsWhenNoJson() + { + // Arrange + ChatMessage message = new(ChatRole.Assistant, +""" + Some text, without JSON +"""); + + // Act + Func action = () => message.ExtractJson(); + + // Assert + action.Should().Throw(); + } + + [Fact] + public void Test_ExtractJson_SuceedsWithQuotesBrackets() + { + // Arrange + ChatMessage message = new(ChatRole.Assistant, +""" + {"reason":"the output contained }", "answer": false} +"""); + + // Act + JsonElement element = message.ExtractJson(); + + // Assert + AnswerReasonPair? result = element.Deserialize(); + + result.Should().NotBeNull(); + result.reason.Should().Be("the output contained }"); + result.answer.Should().BeFalse(); + } + + public static readonly string TestTeamNames = string.Join(", ", ["CodingAgent", "CodeExecutor", "WebSurferAgent", "FileSurferAgent"]); + + [Fact] + public void Test_ProgressLedgerState_IsEmptyWhenStarted() + { + // Arrange/Act + MagenticProgressLedger ledger = new(TestTeamNames, []); + + // Assert + ledger.State.Should().BeNull(); + ledger.IsStarted.Should().BeFalse(); + + ledger.TryGetCurrentSlotValue(TestProgressLedgerState.CustomSlot1, out _).Should().BeFalse(); + ledger.TryGetCurrentSlotValue(TestProgressLedgerState.CustomSlot2, out _).Should().BeFalse(); + } + + [Theory] + [InlineData(0, "RequiredOnly")] + [InlineData(1, "IncludeCustom")] + public void Test_ProgressLedgerState_IsNotEmptyWhenRestored(int caseIndex, string _) + { + // Arrange + TestProgressLedgerState state = TestProgressLedgerState.Working[caseIndex]; + JsonElement element = state.ToJson(); + + // Act + MagenticProgressLedger ledger = new(TestTeamNames, [], element); + + // Assert + ledger.State.Should().Be(element); + state.Validate(ledger); + } + + [Theory] + [InlineData(0, "RequiredOnly")] + [InlineData(1, "IncludeCustom")] + public void Test_ProgressLedgerState_SwitchesToStartedWhenStateUpdates(int caseIndex, string _) + { + // Arrange + MagenticProgressLedger ledger = new(TestTeamNames, []); + TestProgressLedgerState targetState = TestProgressLedgerState.Working[caseIndex]; + JsonElement element = targetState.ToJson(); + ledger.State.Should().BeNull(); + + // Act + ledger.TryUpdateState(element).Should().BeTrue(); + + // Assert + ledger.State.Should().Be(element); + targetState.Validate(ledger); + } + + [Theory] + [InlineData(0, "is_request_satisfied")] + [InlineData(1, "is_in_loop")] + [InlineData(2, "is_progress_being_made")] + [InlineData(3, "instruction_or_question")] + [InlineData(4, "next_speaker")] + public void Test_ProgressLedgerState_FailsToUpdateWhenRequiredAnswersMissing(int caseIndex, string _) + { + // Arrange + MagenticProgressLedger ledger = new(TestTeamNames, []); + TestProgressLedgerState targetState = TestProgressLedgerState.MissingRequired[caseIndex]; + JsonElement element = targetState.ToJson(); + ledger.State.Should().BeNull(); + + // Act + ledger.TryUpdateState(element).Should().BeFalse(); + ledger.State.Should().BeNull(); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void Test_ProgressLedgerState_GeneratesCorrectSchema(bool includeCustom) + { + // Arrange + MagenticProgressLedger ledger = new(TestTeamNames, includeCustom + ? [TestProgressLedgerState.CustomSlot1, TestProgressLedgerState.CustomSlot2] + : []); + + // Act + (string questionBlock, string answerSchema) = ledger.FormatQuestions(); + + foreach (ProgressLedgerSlot slot in ledger.Slots) + { + // Best-efforts validation: I do not want to make it super-brittle and check for 1:1: with the template + // since that is effectively checking that string formatting works right to some extent. + questionBlock.Should().Contain(slot.Question); + answerSchema.Should().Contain(slot.Key); + answerSchema.Should().Contain(slot.SchemaType); + + if (!string.IsNullOrWhiteSpace(slot.SchemaTypeSuffix)) + { + answerSchema.Should().Contain(slot.SchemaTypeSuffix); + } + } + } +} diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/TestProgressLedgerState.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/TestProgressLedgerState.cs new file mode 100644 index 0000000000..fc7e9ded2d --- /dev/null +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/TestProgressLedgerState.cs @@ -0,0 +1,88 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.Text.Json; +using System.Text.Json.Serialization; +using FluentAssertions; + +namespace Microsoft.Agents.AI.Workflows.UnitTests; + +public sealed record Slot(T? answer, string? reason); + +public record TestProgressLedgerState(Slot? is_request_satisfied = null, + Slot? is_in_loop = null, + Slot? is_progress_being_made = null, + Slot? instruction_or_question = null, + Slot? next_speaker = null, + Slot? custom1 = null, + Slot? custom2 = null) +{ + public TestProgressLedgerState() : this(new Slot(false, "is_request_satisfied_reason"), + new Slot(false, "is_in_loop_reason"), + new Slot(false, "is_progress_being_made_reason"), + new Slot("Answer", "instruction_or_question_reason"), + new Slot("Lorem Ipsum", "next_speaker_reason"), + new Slot(false, "custom1_reason"), + new Slot("Custom2", "custom2_reason")) + { } + + public string ToJsonString() => this.ToJson().ToString(); + + private static readonly JsonSerializerOptions s_options = new() + { + DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull + }; + + public JsonElement ToJson() => JsonSerializer.SerializeToElement(this, s_options); + + internal static BooleanProgressLedgerSlot CustomSlot1 = new("custom1", "Custom Slot 1"); + internal static StringProgressLedgerSlot CustomSlot2 = new("custom2", "Custom Slot 2"); + + public static bool TryGetCustom1(MagenticProgressLedger state, out bool result) + => state.TryGetCurrentSlotValue(CustomSlot1, out result); + + public static bool TryGetCustom2(MagenticProgressLedger state, out string? result) + => state.TryGetCurrentSlotValue(CustomSlot2, out result); + + public void Validate(MagenticProgressLedger state) + { + state.IsRequestSatisfied.Should().Be(this.is_request_satisfied!.answer!.Value); + state.IsInLoop.Should().Be(this.is_in_loop!.answer!.Value); + state.IsProgressBeingMade.Should().Be(this.is_progress_being_made!.answer!.Value); + state.InstructionOrQuestion.Should().Be(this.instruction_or_question!.answer); + state.NextSpeaker.Should().Be(this.next_speaker!.answer); + + if (this.custom1 != null) + { + TryGetCustom1(state, out bool custom1Value).Should().BeTrue(); + custom1Value.Should().Be(this.custom1.answer!.Value); + } + else + { + TryGetCustom1(state, out _).Should().BeFalse(); + } + + if (this.custom2 != null) + { + TryGetCustom2(state, out string? custom2Value).Should().BeTrue(); + custom2Value.Should().Be(this.custom2.answer); + } + else + { + TryGetCustom2(state, out _).Should().BeFalse(); + } + } + + public static readonly TestProgressLedgerState Default = new(); + public static readonly TestProgressLedgerState RequiredOnly = Default with { custom1 = null, custom2 = null }; + + public static readonly TestProgressLedgerState[] Working = [RequiredOnly, Default]; + + public static readonly TestProgressLedgerState[] MissingRequired = + [ + Default with { is_request_satisfied = null }, + Default with { is_in_loop = null}, + Default with { is_progress_being_made = null}, + Default with { instruction_or_question = null}, + Default with { next_speaker = null}, + ]; +} diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/TestReplayAgent.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/TestReplayAgent.cs index 3a117b9492..8373b844ed 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/TestReplayAgent.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/TestReplayAgent.cs @@ -11,8 +11,14 @@ using Microsoft.Extensions.AI; namespace Microsoft.Agents.AI.Workflows.UnitTests; -public class TestReplayAgent(List? messages = null, string? id = null, string? name = null) : AIAgent +public class TestReplayAgent(List> messages, string? id = null, string? name = null) : AIAgent { + public TestReplayAgent(List messages, string? id = null, string? name = null) : this([messages ?? []], id, name) + { } + + public TestReplayAgent(string? id = null, string? name = null) : this([[]], id, name) + { } + protected override string? IdCore => id; public override string? Name => name; @@ -57,46 +63,55 @@ public class TestReplayAgent(List? messages = null, string? id = nu public static TestReplayAgent FromStrings(params string[] messages) => new(ToChatMessages(messages)); - public List Messages { get; } = Validate(messages) ?? []; + public List> Messages { get; } = Validate(messages) ?? []; protected override Task RunCoreAsync(IEnumerable messages, AgentSession? session = null, AgentRunOptions? options = null, CancellationToken cancellationToken = default) => this.RunStreamingAsync(messages, session, options, cancellationToken).ToAgentResponseAsync(cancellationToken); + public int Turn { get; set; } + protected override async IAsyncEnumerable RunCoreStreamingAsync(IEnumerable messages, AgentSession? session = null, AgentRunOptions? options = null, [EnumeratorCancellation] CancellationToken cancellationToken = default) { string responseId = Guid.NewGuid().ToString("N"); - foreach (ChatMessage message in this.Messages) + + if (this.Turn < this.Messages.Count) { - foreach (AIContent content in message.Contents) + foreach (ChatMessage message in this.Messages[this.Turn++]) { - yield return new AgentResponseUpdate() + foreach (AIContent content in message.Contents) { - AgentId = this.Id, - AuthorName = this.Name, - MessageId = message.MessageId, - ResponseId = responseId, - Contents = [content], - Role = message.Role, - }; + yield return new AgentResponseUpdate() + { + AgentId = this.Id, + AuthorName = this.Name, + MessageId = message.MessageId, + ResponseId = responseId, + Contents = [content], + Role = message.Role, + }; + } } } } - private static List? Validate(List? candidateMessages) + private static List>? Validate(List>? candidateMessages) { - string? currentMessageId = null; + string? lastMessageId = null; - if (candidateMessages is not null) + if (candidateMessages != null) { - foreach (ChatMessage message in candidateMessages) + foreach (List candidateMessagesTurn in candidateMessages) { - if (currentMessageId is null) + foreach (ChatMessage message in candidateMessagesTurn) { - currentMessageId = message.MessageId; - } - else if (currentMessageId == message.MessageId) - { - throw new ArgumentException("Duplicate consecutive message ids"); + if (lastMessageId is null || lastMessageId != message.MessageId) + { + lastMessageId = message.MessageId; + } + else if (lastMessageId == message.MessageId) + { + throw new ArgumentException("Duplicate consecutive message ids"); + } } } } From a478d1b53c595b44d738e266b958dfddc1a1e172 Mon Sep 17 00:00:00 2001 From: Roger Barreto <19890735+rogerbarreto@users.noreply.github.com> Date: Thu, 7 May 2026 19:54:46 +0100 Subject: [PATCH 2/6] .NET: Foundry.Hosting IT: avoid MSB3026 in publish; fix telemetry UT flake (#5689) CI publish step: gate the BuildProjectReferences=false fast-path on an explicit -UsePrebuiltProjectReferences switch (passed by the workflow) instead of marker detection. Adds a preflight error when stale obj/Release/net10.0 outputs would cause CS0579, with actionable recovery instructions. Telemetry UT flake: AgentFrameworkResponseHandlerTelemetryTests was using a plain List for OTel's InMemoryExporter. The exporter writes from background Activity completion callbacks while parallel tests on the same global ActivitySource feed every listener, racing against the assertion's enumeration and throwing 'Collection was modified'. Replaced with a small thread-safe ConcurrentActivityList that locks add/enumerate and returns a snapshot for assertions. --- .github/workflows/dotnet-build-and-test.yml | 10 ++- .../scripts/it-build-image.ps1 | 64 ++++++++++++++++++- ...tFrameworkResponseHandlerTelemetryTests.cs | 54 ++++++++++++---- 3 files changed, 114 insertions(+), 14 deletions(-) diff --git a/.github/workflows/dotnet-build-and-test.yml b/.github/workflows/dotnet-build-and-test.yml index fcfe26fc0e..2d05ac9a02 100644 --- a/.github/workflows/dotnet-build-and-test.yml +++ b/.github/workflows/dotnet-build-and-test.yml @@ -379,6 +379,14 @@ jobs: # We rebuild and push the test container image on every IT run so framework code changes # are picked up; the image tag is content-hashed across the test container source AND its # framework project references, so identical content is a no-op push. + # + # `-UsePrebuiltProjectReferences` opts into the no-rebuild fast path: publish skips + # rebuilding ProjectReferences and consumes the DLLs the prior "Build Foundry hosted IT + # (and its deps)" step already produced. This avoids MSB3026 ("file is being used by + # another process") collisions caused by the previous build's shared-compilation server + # still holding file handles to those DLLs. Safe in CI because the prebuild step ran in + # the same job against the same source. Do not remove the prebuild step (the subsequent + # `dotnet test --no-build` step depends on it too). - name: Build and push Foundry Hosted Agents test container id: build-foundry-hosted-image shell: pwsh @@ -388,7 +396,7 @@ jobs: if ([string]::IsNullOrWhiteSpace($registry)) { throw "IT_HOSTED_AGENT_REGISTRY not set in the integration environment." } - & "${{ github.workspace }}/dotnet/tests/Foundry.Hosting.IntegrationTests/scripts/it-build-image.ps1" -Registry $registry | Tee-Object -FilePath $env:GITHUB_ENV -Append + & "${{ github.workspace }}/dotnet/tests/Foundry.Hosting.IntegrationTests/scripts/it-build-image.ps1" -Registry $registry -UsePrebuiltProjectReferences | Tee-Object -FilePath $env:GITHUB_ENV -Append - name: Run Foundry Hosted Agents Integration Tests shell: pwsh diff --git a/dotnet/tests/Foundry.Hosting.IntegrationTests/scripts/it-build-image.ps1 b/dotnet/tests/Foundry.Hosting.IntegrationTests/scripts/it-build-image.ps1 index 2bb4670a76..857ea3be0a 100644 --- a/dotnet/tests/Foundry.Hosting.IntegrationTests/scripts/it-build-image.ps1 +++ b/dotnet/tests/Foundry.Hosting.IntegrationTests/scripts/it-build-image.ps1 @@ -41,7 +41,14 @@ param( [string] $Repository = "foundry-hosting-it", - [string] $TestContainerProject = "dotnet/tests/Foundry.Hosting.IntegrationTests.TestContainer" + [string] $TestContainerProject = "dotnet/tests/Foundry.Hosting.IntegrationTests.TestContainer", + + # Explicit opt-in for the no-rebuild fast path. CI sets this after running the + # "Build Foundry hosted IT (and its deps)" step, which guarantees the prebuilt + # library DLLs match current source. Off by default so local invocations always + # let publish rebuild ProjectReferences and never produce an image whose tag is + # computed from current source while the contents come from a stale build. + [switch] $UsePrebuiltProjectReferences ) $ErrorActionPreference = "Stop" @@ -100,7 +107,60 @@ if (Test-Path $out) { Remove-Item -Recurse -Force $out } -dotnet publish $TestContainerProject -c Release -f net10.0 -r linux-musl-x64 --self-contained false -o $out --tl:off | Out-Host +# Conditionally tell publish to skip rebuilding ProjectReferences and consume the +# prebuilt library DLLs in place. This avoids two failure modes that arise when +# the CI workflow runs a `dotnet build` of the same library projects immediately +# before this script: +# 1) MSB3026 "file is being used by another process" when publish's MSBuild +# tries to overwrite src//bin/Release/net10.0/.dll while the +# previous build's shared-compilation server still holds a file handle. +# 2) Publish needlessly rebuilding identical managed (RID-agnostic) library +# DLLs that prebuild already produced. +# Gated on -UsePrebuiltProjectReferences (a strict opt-in) instead of marker +# detection, because a developer machine may have a stale Release build of the +# libraries from days ago; using those would silently produce an image whose +# content is older than the source the tag is computed from. +$publishExtraArgs = @() +if ($UsePrebuiltProjectReferences) { + Write-Host "-UsePrebuiltProjectReferences: skipping ProjectReference rebuild." -ForegroundColor DarkGray + $publishExtraArgs += "-p:BuildProjectReferences=false" +} else { + # Preflight: in default (rebuild) mode, publish propagates RuntimeIdentifier=linux-musl-x64 + # to library ProjectReferences and writes their intermediates to a RID-suffixed obj path + # (e.g. obj/Release/net10.0/linux-musl-x64/). DefaultItemExcludes follows the new + # IntermediateOutputPath, so any *.AssemblyInfo.cs left in obj/Release/net10.0/ from a + # prior `dotnet build` is no longer excluded and gets picked up by the **/*.cs Compile + # glob, producing CS0579 "duplicate attribute" errors. Detect that state up front and + # tell the user exactly how to recover. + $staleObjProbes = @( + "dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/obj/Release/net10.0", + "dotnet/src/Microsoft.Agents.AI.Foundry/obj/Release/net10.0", + "dotnet/src/Microsoft.Agents.AI/obj/Release/net10.0", + "dotnet/src/Microsoft.Agents.AI.Abstractions/obj/Release/net10.0" + ) + $stale = @($staleObjProbes | Where-Object { Test-Path (Join-Path $_ "*.AssemblyInfo.cs") }) + if ($stale.Count -gt 0) { + $msg = @( + "Detected prior Release/net10.0 build outputs in:" + ($stale | ForEach-Object { " - $_" }) + "" + "Publish would propagate -r linux-musl-x64 to those ProjectReferences and the" + "leftover obj/Release/net10.0/*.AssemblyInfo.cs files would cause CS0579 duplicate" + "attribute errors. Pick one:" + " (a) Pass -UsePrebuiltProjectReferences (skips ProjectReference rebuild and" + " uses the existing src//bin/Release/net10.0/*.dll outputs in place)." + " Only safe when you know those DLLs match current source - this is the path" + " CI uses immediately after its 'Build Foundry hosted IT (and its deps)' step." + " (b) Remove the stale obj/Release trees, e.g.:" + " Remove-Item -Recurse -Force dotnet/src/Microsoft.Agents.AI*/obj/Release" + " and re-run." + ) -join "`n" + throw $msg + } + Write-Host "Letting publish build ProjectReferences (pass -UsePrebuiltProjectReferences in CI to skip)." -ForegroundColor DarkGray +} + +dotnet publish $TestContainerProject -c Release -f net10.0 -r linux-musl-x64 --self-contained false -o $out @publishExtraArgs --tl:off | Out-Host if ($LASTEXITCODE -ne 0) { throw "dotnet publish failed with exit code $LASTEXITCODE." } diff --git a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/AgentFrameworkResponseHandlerTelemetryTests.cs b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/AgentFrameworkResponseHandlerTelemetryTests.cs index dcb4e7e212..360c065eee 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/AgentFrameworkResponseHandlerTelemetryTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/AgentFrameworkResponseHandlerTelemetryTests.cs @@ -37,7 +37,7 @@ public class AgentFrameworkResponseHandlerTelemetryTests public async Task CreateAsync_DefaultAgent_EmitsInvokeAgentSpanAsync() { // Arrange - var activities = new List(); + var activities = new ConcurrentActivityList(); using var tracerProvider = Sdk.CreateTracerProviderBuilder() .AddSource(ResponsesSourceName) .AddInMemoryExporter(activities) @@ -56,7 +56,7 @@ public class AgentFrameworkResponseHandlerTelemetryTests await foreach (var _ in handler.CreateAsync(request, context, CancellationToken.None)) { } // Assert — filter by agent name to isolate this test's span from any parallel test spans - var mySpan = Assert.Single(activities.Where(a => TelemetryTestAgent.AgentName.Equals(a.GetTagItem("gen_ai.agent.name"))).ToList()); + var mySpan = Assert.Single(activities.Snapshot().Where(a => TelemetryTestAgent.AgentName.Equals(a.GetTagItem("gen_ai.agent.name"))).ToList()); Assert.Equal("invoke_agent", mySpan.GetTagItem("gen_ai.operation.name")); Assert.NotNull(mySpan.GetTagItem("gen_ai.agent.id")); } @@ -65,7 +65,7 @@ public class AgentFrameworkResponseHandlerTelemetryTests public async Task CreateAsync_KeyedAgent_EmitsInvokeAgentSpanAsync() { // Arrange - var activities = new List(); + var activities = new ConcurrentActivityList(); using var tracerProvider = Sdk.CreateTracerProviderBuilder() .AddSource(ResponsesSourceName) .AddInMemoryExporter(activities) @@ -84,7 +84,7 @@ public class AgentFrameworkResponseHandlerTelemetryTests await foreach (var _ in handler.CreateAsync(request, context, CancellationToken.None)) { } // Assert — filter by agent name to isolate this test's span - var mySpan = Assert.Single(activities.Where(a => TelemetryTestAgent.AgentName.Equals(a.GetTagItem("gen_ai.agent.name"))).ToList()); + var mySpan = Assert.Single(activities.Snapshot().Where(a => TelemetryTestAgent.AgentName.Equals(a.GetTagItem("gen_ai.agent.name"))).ToList()); Assert.Equal("invoke_agent", mySpan.GetTagItem("gen_ai.operation.name")); } @@ -95,8 +95,8 @@ public class AgentFrameworkResponseHandlerTelemetryTests // If ApplyOpenTelemetry double-wraps, an extra span would appear on ResponsesSourceName. // If it correctly skips wrapping, only the pre-wrap's unique source emits spans. var preWrapSource = Guid.NewGuid().ToString(); - var preWrapActivities = new List(); - var responsesActivities = new List(); + var preWrapActivities = new ConcurrentActivityList(); + var responsesActivities = new ConcurrentActivityList(); using var preWrapProvider = Sdk.CreateTracerProviderBuilder() .AddSource(preWrapSource) @@ -125,18 +125,19 @@ public class AgentFrameworkResponseHandlerTelemetryTests await foreach (var _ in handler.CreateAsync(request, context, CancellationToken.None)) { } // Assert — pre-wrap source emits exactly 1 span (agent ran) - Assert.Single(preWrapActivities); - Assert.Equal("invoke_agent", preWrapActivities[0].GetTagItem("gen_ai.operation.name")); + var preWrapSnapshot = preWrapActivities.Snapshot(); + Assert.Single(preWrapSnapshot); + Assert.Equal("invoke_agent", preWrapSnapshot[0].GetTagItem("gen_ai.operation.name")); // ResponsesSourceName emits 0 spans — ApplyOpenTelemetry skipped wrapping the pre-instrumented agent - Assert.DoesNotContain(responsesActivities, a => TelemetryTestAgent.AgentName.Equals(a.GetTagItem("gen_ai.agent.name"))); + Assert.DoesNotContain(responsesActivities.Snapshot(), a => TelemetryTestAgent.AgentName.Equals(a.GetTagItem("gen_ai.agent.name"))); } [Fact] public async Task CreateAsync_DefaultAgent_SpanDisplayNameContainsAgentNameAsync() { // Arrange - var activities = new List(); + var activities = new ConcurrentActivityList(); using var tracerProvider = Sdk.CreateTracerProviderBuilder() .AddSource(ResponsesSourceName) .AddInMemoryExporter(activities) @@ -155,7 +156,7 @@ public class AgentFrameworkResponseHandlerTelemetryTests await foreach (var _ in handler.CreateAsync(request, context, CancellationToken.None)) { } // Assert — display name follows "invoke_agent {Name}({Id})" convention; filter by agent name to isolate - var mySpan = Assert.Single(activities.Where(a => TelemetryTestAgent.AgentName.Equals(a.GetTagItem("gen_ai.agent.name"))).ToList()); + var mySpan = Assert.Single(activities.Snapshot().Where(a => TelemetryTestAgent.AgentName.Equals(a.GetTagItem("gen_ai.agent.name"))).ToList()); Assert.Contains("invoke_agent", mySpan.DisplayName, StringComparison.Ordinal); Assert.Contains(TelemetryTestAgent.AgentName, mySpan.DisplayName, StringComparison.Ordinal); } @@ -231,4 +232,35 @@ public class AgentFrameworkResponseHandlerTelemetryTests } private sealed class TelemetryAgentSession : AgentSession; + + /// + /// Thread-safe used by OTel's InMemoryExporter to capture + /// activities emitted on globally-listened sources. Required because the exporter writes into + /// the supplied collection from background Activity completion callbacks while the test thread + /// may be enumerating it for assertions, and other tests in the same assembly may emit on the + /// same source concurrently. A plain trips + /// "Collection was modified; enumeration operation may not execute." in that scenario. + /// + private sealed class ConcurrentActivityList : ICollection + { + private readonly List _items = new(); + private readonly object _gate = new(); + + public int Count { get { lock (this._gate) { return this._items.Count; } } } + public bool IsReadOnly => false; + + public void Add(Activity item) { lock (this._gate) { this._items.Add(item); } } + public void Clear() { lock (this._gate) { this._items.Clear(); } } + public bool Contains(Activity item) { lock (this._gate) { return this._items.Contains(item); } } + public void CopyTo(Activity[] array, int arrayIndex) { lock (this._gate) { this._items.CopyTo(array, arrayIndex); } } + public bool Remove(Activity item) { lock (this._gate) { return this._items.Remove(item); } } + + public Activity[] Snapshot() + { + lock (this._gate) { return this._items.ToArray(); } + } + + public IEnumerator GetEnumerator() => ((IEnumerable)this.Snapshot()).GetEnumerator(); + System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => this.GetEnumerator(); + } } From 1d94518f3799f092870d0a02972e0b62c3e59067 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh <68852919+SergeyMenshykh@users.noreply.github.com> Date: Thu, 7 May 2026 20:39:12 +0100 Subject: [PATCH 3/6] Python: Add ClassSkill for class-based skill definitions (#5678) * Python: Add ClassSkill for class-based skill definitions Add ClassSkill abstract base class with decorator-based resource and script discovery, porting .NET's AgentClassSkill (PRs #5027 and #5183) to Python. - Add ClassSkill(Skill, ABC) with instructions abstract property, cached content/resources/scripts properties - Add @ClassSkill.resource and @ClassSkill.script static method decorators for auto-discovery of methods and properties - Extract _build_skill_content() and _create_resource_element() shared helpers from InlineSkill for reuse - Add _discover_marked_members() for scanning class hierarchies - Add _make_method_name() for Python-to-skill name conversion - Add class_based_skill sample (UnitConverterSkill) - Update mixed_skills sample with TemperatureConverterSkill - Add 58 new tests covering ClassSkill, decorator discovery, property resources, inheritance, kwargs forwarding, and duplicate detection - Export ClassSkill from agent_framework public API Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: replace try/except/continue with assignment to satisfy bandit B112 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * address PR review feedback - Walk cls.__mro__ in _discover_marked_members for inherited property resources - Use inspect.getattr_static for MRO-aware is_property check - Return defensive copies from resources/scripts properties - Raise TypeError on wrong decorator stacking order (@resource above @property) - Log warning instead of silently swallowing descriptor errors during discovery - Validate explicit name= at decoration time via _validate_member_name - Add tests for all of the above Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix temperature converter skill: make resource necessary for script Refactor TemperatureConverterSkill so the agent must read the formulas resource (factor/offset) before calling the script, aligning with the volume-converter pattern. - Resource: numeric factor/offset table instead of symbolic formulas - Script: generic linear transform (value * factor + offset) - Instructions: updated to reflect new workflow Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../packages/core/agent_framework/__init__.py | 2 + .../packages/core/agent_framework/_skills.py | 525 +++++++++++- .../packages/core/tests/core/test_skills.py | 770 +++++++++++++++++- python/samples/02-agents/skills/README.md | 22 +- .../skills/class_based_skill/README.md | 71 ++ .../class_based_skill/class_based_skill.py | 145 ++++ .../02-agents/skills/mixed_skills/README.md | 26 +- .../skills/mixed_skills/mixed_skills.py | 106 ++- 8 files changed, 1583 insertions(+), 84 deletions(-) create mode 100644 python/samples/02-agents/skills/class_based_skill/README.md create mode 100644 python/samples/02-agents/skills/class_based_skill/class_based_skill.py diff --git a/python/packages/core/agent_framework/__init__.py b/python/packages/core/agent_framework/__init__.py index 4592f8c716..db1c43abfe 100644 --- a/python/packages/core/agent_framework/__init__.py +++ b/python/packages/core/agent_framework/__init__.py @@ -135,6 +135,7 @@ from ._sessions import ( from ._settings import SecretString, load_settings from ._skills import ( AggregatingSkillsSource, + ClassSkill, DeduplicatingSkillsSource, DelegatingSkillsSource, FileSkill, @@ -345,6 +346,7 @@ __all__ = [ "ChatResponseUpdate", "CheckResult", "CheckpointStorage", + "ClassSkill", "CompactionProvider", "CompactionStrategy", "Content", diff --git a/python/packages/core/agent_framework/_skills.py b/python/packages/core/agent_framework/_skills.py index 06612b4df0..a1d5634e18 100644 --- a/python/packages/core/agent_framework/_skills.py +++ b/python/packages/core/agent_framework/_skills.py @@ -5,7 +5,7 @@ Defines the core data model classes for the agent skills system: - **Skills:** :class:`Skill` (abstract base), :class:`InlineSkill` (code-defined), - and :class:`FileSkill` (filesystem-backed). + :class:`ClassSkill` (class-based), and :class:`FileSkill` (filesystem-backed). - **Resources:** :class:`SkillResource` (abstract base), :class:`InlineSkillResource` (static content or callable). - **Scripts:** :class:`SkillScript` (abstract base), :class:`InlineSkillScript` @@ -27,6 +27,9 @@ Skills can come from different sources: Represented as :class:`FileSkill` instances. - **Code-defined** — created as :class:`InlineSkill` instances in Python code, with optional callable resources attached via the ``@skill.resource`` decorator. +- **Class-based** — created by subclassing :class:`ClassSkill` to define + self-contained, reusable skill types with ``create_resource()`` and + ``create_script()`` factory methods. - **Custom sources** — any :class:`SkillsSource` implementation that provides skills from arbitrary origins (REST APIs, databases, etc.). @@ -570,6 +573,65 @@ def _validate_skill_description(name: str, description: str) -> None: ) +def _build_skill_content( + name: str, + description: str, + instructions: str, + resources: Sequence[SkillResource] | None = None, + scripts: Sequence[SkillScript] | None = None, +) -> str: + """Build XML-structured content for code-defined and class-based skills. + + Produces an XML document containing name, description, instructions, + resources, and scripts elements. Used by both :class:`InlineSkill` + and :class:`ClassSkill` to generate their ``content`` property. + + Args: + name: The skill name. + description: The skill description. + instructions: The raw instructions text. + resources: Optional resources associated with the skill. + scripts: Optional scripts associated with the skill. + + Returns: + An XML-structured content string. + """ + result = ( + f"{xml_escape(name)}\n" + f"{xml_escape(description)}\n" + "\n" + "\n" + f"{instructions}\n" + "" + ) + + if resources: + resource_lines = "\n".join(_create_resource_element(r) for r in resources) + result += f"\n\n\n{resource_lines}\n" + + if scripts: + script_lines = "\n".join(_create_script_element(s) for s in scripts) + result += f"\n\n\n{script_lines}\n" + + return result + + +def _create_resource_element(resource: SkillResource) -> str: + """Create a self-closing ```` XML element from a :class:`SkillResource`. + + Args: + resource: The resource to create the element from. + + Returns: + A single indented XML element string with ``name`` and optional + ``description`` attributes. + """ + attrs = f'name="{xml_escape(resource.name, quote=True)}"' + if resource.description: + attrs += f' description="{xml_escape(resource.description, quote=True)}"' + return f" " + + @experimental(feature_id=ExperimentalFeature.SKILLS) class InlineSkill(Skill): """A skill defined entirely in code with resources and scripts. @@ -634,25 +696,10 @@ class InlineSkill(Skill): if self._cached_content is not None: return self._cached_content - result = ( - f"{xml_escape(self.name)}\n" - f"{xml_escape(self.description)}\n" - "\n" - "\n" - f"{self.instructions}\n" - "" + self._cached_content = _build_skill_content( + self.name, self.description, self.instructions, self._resources, self._scripts ) - - if self._resources: - resource_lines = "\n".join(self._create_resource_element(r) for r in self._resources) - result += f"\n\n\n{resource_lines}\n" - - if self._scripts: - script_lines = "\n".join(_create_script_element(s) for s in self._scripts) - result += f"\n\n\n{script_lines}\n" - - self._cached_content = result - return result + return self._cached_content @property def resources(self) -> list[SkillResource]: @@ -664,22 +711,6 @@ class InlineSkill(Skill): """Mutable list of :class:`SkillScript` instances.""" return self._scripts - @staticmethod - def _create_resource_element(resource: SkillResource) -> str: - """Create a self-closing ```` XML element from an :class:`SkillResource`. - - Args: - resource: The resource to create the element from. - - Returns: - A single indented XML element string with ``name`` and optional - ``description`` attributes. - """ - attrs = f'name="{xml_escape(resource.name, quote=True)}"' - if resource.description: - attrs += f' description="{xml_escape(resource.description, quote=True)}"' - return f" " - def resource( self, func: Callable[..., Any] | None = None, @@ -700,8 +731,7 @@ class InlineSkill(Skill): Keyword Args: name: Resource name override. Defaults to ``func.__name__``. - description: Resource description override. Defaults to the - function's docstring (via :func:`inspect.getdoc`). + description: Resource description override. Defaults to ``None``. Returns: The original function unchanged, or a secondary decorator when @@ -727,7 +757,7 @@ class InlineSkill(Skill): def decorator(f: Callable[..., Any]) -> Callable[..., Any]: resource_name = name or f.__name__ - resource_description = description or (inspect.getdoc(f) or None) + resource_description = description self._resources.append( InlineSkillResource( name=resource_name, @@ -761,8 +791,7 @@ class InlineSkill(Skill): Keyword Args: name: Script name override. Defaults to ``func.__name__``. - description: Script description override. Defaults to the - function's docstring (via :func:`inspect.getdoc`). + description: Script description override. Defaults to ``None``. Returns: The original function unchanged, or a secondary decorator when @@ -789,7 +818,7 @@ class InlineSkill(Skill): def decorator(f: Callable[..., Any]) -> Callable[..., Any]: script_name = name or f.__name__ - script_description = description or (inspect.getdoc(f) or None) + script_description = description self._scripts.append( InlineSkillScript( name=script_name, @@ -804,6 +833,420 @@ class InlineSkill(Skill): return decorator(func) +def _make_method_name(method_name: str) -> str: + """Convert a Python method name to a skill resource/script name. + + Replaces underscores with hyphens to match the skill naming convention. + + Args: + method_name: The Python method name (e.g. ``"conversion_table"``). + + Returns: + The converted name (e.g. ``"conversion-table"``). + """ + return method_name.replace("_", "-").strip("-") + + +def _validate_member_name(name: str, kind: str) -> None: + """Validate a resource or script name at decoration time. + + Args: + name: The name to validate. + kind: ``"resource"`` or ``"script"`` — used in error messages. + + Raises: + ValueError: If the name is empty, too long, or contains invalid characters. + """ + if not name or not name.strip(): + raise ValueError(f"@ClassSkill.{kind} name cannot be empty.") + if len(name) > MAX_NAME_LENGTH or not VALID_NAME_RE.match(name): + raise ValueError( + f"Invalid @ClassSkill.{kind} name '{name}': Must be {MAX_NAME_LENGTH} characters or fewer, " + "using only lowercase letters, numbers, and hyphens, and must not start or end with a hyphen " + "or contain consecutive hyphens." + ) + + +def _discover_marked_members(cls: type, marker_attr: str) -> list[tuple[str, dict[str, Any]]]: + """Scan a class for methods or properties stamped with a marker attribute. + + Checks both regular callable attributes (via ``dir``) and ``property`` + descriptors (via ``cls.__dict__``) whose ``fget`` carries the marker. + + Args: + cls: The class to scan. + marker_attr: The marker attribute name to look for (e.g. + ``"_skill_resource_marker"``). + + Returns: + A list of ``(member_name, marker_dict)`` tuples. + """ + results: list[tuple[str, dict[str, Any]]] = [] + seen: set[str] = set() + + # Walk the MRO so that property-resources defined on a parent class + # are also discovered. ``cls.__dict__`` only sees the leaf class. + for klass in cls.__mro__: + for attr_name, attr_value in klass.__dict__.items(): + if attr_name in seen: + continue + if ( + isinstance(attr_value, property) + and attr_value.fget is not None + and hasattr(attr_value.fget, marker_attr) + ): + results.append((attr_name, getattr(attr_value.fget, marker_attr))) + seen.add(attr_name) + + # Check regular callable attributes. + for attr_name in dir(cls): + if attr_name in seen: + continue + try: + attr = getattr(cls, attr_name, None) + except Exception: + # Some descriptors (e.g. abstract properties) may raise on access. + logger.warning("Skipping '%s' during skill discovery: descriptor raised on access", attr_name) + attr = None + if attr is not None and callable(attr) and hasattr(attr, marker_attr): + results.append((attr_name, getattr(attr, marker_attr))) + return results + + +@experimental(feature_id=ExperimentalFeature.SKILLS) +class ClassSkill(Skill, ABC): + """Abstract base class for defining skills as reusable Python classes. + + Inherit from this class to create a self-contained skill definition. + Override :attr:`instructions` to provide the skill body. + + Resources and scripts can be defined in two ways: + + - **Decorator-based (recommended):** Mark methods with + :meth:`ClassSkill.resource` and :meth:`ClassSkill.script` decorators + for automatic discovery. + - **Explicit override:** Override the :attr:`resources` and + :attr:`scripts` properties, constructing :class:`InlineSkillResource` + and :class:`InlineSkillScript` instances directly. + + Class-based skills can be distributed via shared libraries or PyPI + packages, making them easy to reuse across projects. + + Attributes: + name: Skill name (lowercase letters, numbers, hyphens only). + description: Human-readable description of the skill. + + Examples: + Decorator-based (recommended): + + .. code-block:: python + + class UnitConverterSkill(ClassSkill): + def __init__(self) -> None: + super().__init__( + name="unit-converter", + description="Convert between common units.", + ) + + @property + def instructions(self) -> str: + return "Use this skill to convert units..." + + @ClassSkill.resource(name="table") + def conversion_table(self) -> str: + return "| From | To | Factor |..." + + @ClassSkill.script(name="convert") + def convert(self, value: float, factor: float) -> str: + return json.dumps({"result": round(value * factor, 4)}) + + Explicit override: + + .. code-block:: python + + class UnitConverterSkill(ClassSkill): + def __init__(self) -> None: + super().__init__( + name="unit-converter", + description="Convert between common units.", + ) + + @property + def instructions(self) -> str: + return "Use this skill to convert units..." + + @property + def resources(self) -> list[SkillResource]: + return [ + InlineSkillResource(name="table", content="| From | To | Factor |..."), + ] + + @property + def scripts(self) -> list[SkillScript]: + return [InlineSkillScript(name="convert", function=convert_fn)] + """ + + def __init__( + self, + *, + name: str, + description: str, + ) -> None: + """Initialize a ClassSkill. + + Args: + name: Skill name (lowercase letters, numbers, hyphens only; + max 64 characters). + description: Human-readable description of the skill + (≤1024 characters). + """ + super().__init__(name=name, description=description) + self._cached_content: str | None = None + self._cached_resources: list[SkillResource] | None = None + self._cached_scripts: list[SkillScript] | None = None + + @staticmethod + def resource( + func: Callable[..., Any] | None = None, + *, + name: str | None = None, + description: str | None = None, + ) -> Any: + """Decorator that marks a method or property as a skill resource for auto-discovery. + + When applied to a method or property on a :class:`ClassSkill` subclass, + it is automatically discovered and registered as an + :class:`InlineSkillResource`. Methods are invoked each time the + resource is read. Properties are evaluated via their getter. + + Can be applied to a method directly, or stacked with ``@property`` + (place ``@property`` first, ``@ClassSkill.resource`` second). + + Supports bare usage (``@ClassSkill.resource``) and parameterized usage + (``@ClassSkill.resource(name="custom", description="...")``). + + Args: + func: The function being decorated. Populated automatically when + the decorator is applied without parentheses. + + Keyword Args: + name: Resource name override. Defaults to the method name with + underscores replaced by hyphens. + description: Resource description. Defaults to ``None``. + + Examples: + On a method: + + .. code-block:: python + + @ClassSkill.resource(name="conversion-table") + def get_table(self) -> str: + return "..." + + On a property: + + .. code-block:: python + + @property + @ClassSkill.resource + def conversion_table(self) -> str: + return "..." + """ + + def decorator(f: Callable[..., Any]) -> Callable[..., Any]: + if isinstance(f, (property, classmethod, staticmethod)): + raise TypeError( + "@ClassSkill.resource must be applied before @property, @classmethod, or @staticmethod. " + "Place @property first, then @ClassSkill.resource." + ) + if name is not None: + _validate_member_name(name, "resource") + f._skill_resource_marker = { # type: ignore[attr-defined] + "name": name, + "description": description, + } + return f + + if func is None: + return decorator + return decorator(func) + + @staticmethod + def script( + func: Callable[..., Any] | None = None, + *, + name: str | None = None, + description: str | None = None, + ) -> Any: + """Decorator that marks a method as a skill script for auto-discovery. + + When applied to a method on a :class:`ClassSkill` subclass, the method is + automatically discovered and registered as an :class:`InlineSkillScript`. + The method's parameters (excluding ``self``) are used to generate a JSON + schema, and the method is invoked in-process when the script is run. + + Supports bare usage (``@ClassSkill.script``) and parameterized usage + (``@ClassSkill.script(name="custom", description="...")``). + + Args: + func: The function being decorated. Populated automatically when + the decorator is applied without parentheses. + + Keyword Args: + name: Script name override. Defaults to the method name with + underscores replaced by hyphens. + description: Script description. Defaults to ``None``. + + Examples: + .. code-block:: python + + @ClassSkill.script(name="convert") + def convert(self, value: float, factor: float) -> str: + return json.dumps({"result": round(value * factor, 4)}) + """ + + def decorator(f: Callable[..., Any]) -> Callable[..., Any]: + if isinstance(f, (property, classmethod, staticmethod)): + raise TypeError( + "@ClassSkill.script must be applied before @property, @classmethod, or @staticmethod." + ) + if name is not None: + _validate_member_name(name, "script") + f._skill_script_marker = { # type: ignore[attr-defined] + "name": name, + "description": description, + } + return f + + if func is None: + return decorator + return decorator(func) + + @property + @abstractmethod + def instructions(self) -> str: + """The raw instructions text for this skill. + + Subclasses must override this property to provide the skill body. + """ + ... + + @property + def resources(self) -> list[SkillResource]: + """Resources discovered from :meth:`ClassSkill.resource`-decorated methods. + + On first access, scans the class for methods marked with the + :meth:`ClassSkill.resource` decorator and instantiates + :class:`InlineSkillResource` instances from them. + The result is cached after the first access. + + Override this property to provide resources explicitly instead of + using decorator-based discovery. + """ + if self._cached_resources is not None: + return list(self._cached_resources) + + resources: list[SkillResource] = [] + seen_names: set[str] = set() + + for attr_name, attr in _discover_marked_members(type(self), "_skill_resource_marker"): + marker: dict[str, Any] = attr + resource_name = marker.get("name") or _make_method_name(attr_name) + if resource_name in seen_names: + raise ValueError( + f"Skill '{self.name}' already has a resource named '{resource_name}'. " + "Ensure each @ClassSkill.resource has a unique name." + ) + seen_names.add(resource_name) + + # Use inspect.getattr_static to check the descriptor type without + # triggering it, and walk the MRO so inherited properties are found. + static_attr = inspect.getattr_static(self, attr_name, None) + is_property = isinstance(static_attr, property) + resource_description = marker.get("description") + + if is_property: + # Property — use a lambda that reads the property value each time. + # We capture attr_name to avoid late-binding issues. + # Do NOT call getattr here to avoid triggering the getter during discovery. + resource_func = (lambda name: lambda: getattr(self, name))(attr_name) + resources.append( + InlineSkillResource( + name=resource_name, + function=resource_func, + description=resource_description, + ) + ) + else: + # Regular method — use the bound method directly. + bound_method = getattr(self, attr_name) + resources.append( + InlineSkillResource( + name=resource_name, + function=bound_method, + description=resource_description, + ) + ) + + self._cached_resources = resources + return list(self._cached_resources) + + @property + def scripts(self) -> list[SkillScript]: + """Scripts discovered from :meth:`ClassSkill.script`-decorated methods. + + On first access, scans the class for methods marked with the + :meth:`ClassSkill.script` decorator and instantiates + :class:`InlineSkillScript` instances from them. + The result is cached after the first access. + + Override this property to provide scripts explicitly instead of + using decorator-based discovery. + """ + if self._cached_scripts is not None: + return list(self._cached_scripts) + + scripts: list[SkillScript] = [] + seen_names: set[str] = set() + + for attr_name, attr in _discover_marked_members(type(self), "_skill_script_marker"): + marker: dict[str, Any] = attr + script_name = marker.get("name") or _make_method_name(attr_name) + if script_name in seen_names: + raise ValueError( + f"Skill '{self.name}' already has a script named '{script_name}'. " + "Ensure each @ClassSkill.script has a unique name." + ) + seen_names.add(script_name) + + bound_method = getattr(self, attr_name) + script_description = marker.get("description") + scripts.append( + InlineSkillScript( + name=script_name, + function=bound_method, + description=script_description, + ) + ) + + self._cached_scripts = scripts + return list(self._cached_scripts) + + @property + def content(self) -> str: + """Synthesized XML content containing name, description, instructions, resources, and scripts. + + The result is cached after the first access. + """ + if self._cached_content is not None: + return self._cached_content + + self._cached_content = _build_skill_content( + self.name, self.description, self.instructions, self.resources, self.scripts + ) + return self._cached_content + + @experimental(feature_id=ExperimentalFeature.SKILLS) class FileSkill(Skill): """A :class:`Skill` discovered from a filesystem directory backed by a SKILL.md file. diff --git a/python/packages/core/tests/core/test_skills.py b/python/packages/core/tests/core/test_skills.py index 8e8c6a8aed..b268b31551 100644 --- a/python/packages/core/tests/core/test_skills.py +++ b/python/packages/core/tests/core/test_skills.py @@ -5,6 +5,7 @@ from __future__ import annotations import os +from abc import ABC from collections.abc import Sequence from pathlib import Path from typing import Any @@ -14,6 +15,7 @@ import pytest from agent_framework import ( AggregatingSkillsSource, + ClassSkill, DeduplicatingSkillsSource, FileSkill, FileSkillScript, @@ -32,6 +34,7 @@ from agent_framework._skills import ( DEFAULT_SCRIPT_EXTENSIONS, InlineSkillResource, InlineSkillScript, + _create_resource_element, _create_script_element, _FileSkillResource, ) @@ -1004,7 +1007,7 @@ class TestInlineSkill: assert len(skill.resources) == 1 assert skill.resources[0].name == "get_schema" - assert skill.resources[0].description == "Get the database schema." + assert skill.resources[0].description is None assert isinstance(skill.resources[0], InlineSkillResource) assert skill.resources[0].function is get_schema @@ -1677,22 +1680,22 @@ class TestCreateResourceElement: def test_name_only(self) -> None: r = InlineSkillResource(name="my-ref", content="data") - elem = InlineSkill._create_resource_element(r) + elem = _create_resource_element(r) assert elem == ' ' def test_with_description(self) -> None: r = InlineSkillResource(name="my-ref", description="A reference.", content="data") - elem = InlineSkill._create_resource_element(r) + elem = _create_resource_element(r) assert elem == ' ' def test_xml_escapes_name(self) -> None: r = InlineSkillResource(name='ref"special', content="data") - elem = InlineSkill._create_resource_element(r) + elem = _create_resource_element(r) assert """ in elem def test_xml_escapes_description(self) -> None: r = InlineSkillResource(name="ref", description='Uses & "quotes"', content="data") - elem = InlineSkill._create_resource_element(r) + elem = _create_resource_element(r) assert "<tags>" in elem assert "&" in elem assert """ in elem @@ -2136,8 +2139,8 @@ class TestSkillResourceDecoratorEdgeCases: return "data" assert skill.resources[0].name == "custom-name" - # description falls back to docstring - assert skill.resources[0].description == "Some docs." + # description is None when not explicitly provided + assert skill.resources[0].description is None def test_decorator_with_description_only(self) -> None: skill = InlineSkill(name="my-skill", description="A skill.", instructions="Body") @@ -2320,7 +2323,7 @@ class TestSkillScriptDecorator: assert len(skill.scripts) == 1 assert skill.scripts[0].name == "analyze" - assert skill.scripts[0].description == "Run analysis." + assert skill.scripts[0].description is None assert isinstance(skill.scripts[0], InlineSkillScript) assert skill.scripts[0].function is analyze @@ -3177,6 +3180,757 @@ class TestLoadSkillWithScripts: result = provider._load_skill(_raw_skills(provider), "my-skill") assert "" not in result + +# --------------------------------------------------------------------------- +# Tests: ClassSkill +# --------------------------------------------------------------------------- + + +class _MinimalClassSkill(ClassSkill): + """A minimal class-based skill with no resources or scripts.""" + + def __init__(self) -> None: + super().__init__(name="minimal-skill", description="A minimal skill.") + + @property + def instructions(self) -> str: + return "Do minimal things." + + +class _FullClassSkill(ClassSkill): + """A class-based skill with resources and scripts.""" + + def __init__(self) -> None: + super().__init__(name="full-skill", description="A full skill.") + self._resources: list[SkillResource] | None = None + self._scripts: list[SkillScript] | None = None + + @property + def instructions(self) -> str: + return "Use this skill for full tasks." + + @property + def resources(self) -> list[SkillResource]: + if self._resources is None: + self._resources = [ + InlineSkillResource(name="test-resource", content="Static resource content."), + ] + return self._resources + + @property + def scripts(self) -> list[SkillScript]: + if self._scripts is None: + self._scripts = [ + InlineSkillScript(name="test-script", function=_class_skill_test_fn), + ] + return self._scripts + + +def _class_skill_test_fn(value: float, factor: float) -> str: + """Multiply value by factor.""" + import json as _json + + return _json.dumps({"result": round(value * factor, 4)}) + + +class TestClassSkill: + """Tests for ClassSkill abstract base class.""" + + def test_minimal_skill_has_no_resources(self) -> None: + skill = _MinimalClassSkill() + assert skill.resources == [] + + def test_minimal_skill_has_no_scripts(self) -> None: + skill = _MinimalClassSkill() + assert skill.scripts == [] + + def test_minimal_skill_content_contains_name(self) -> None: + skill = _MinimalClassSkill() + assert "minimal-skill" in skill.content + + def test_minimal_skill_content_contains_description(self) -> None: + skill = _MinimalClassSkill() + assert "A minimal skill." in skill.content + + def test_minimal_skill_content_contains_instructions(self) -> None: + skill = _MinimalClassSkill() + assert "Do minimal things." in skill.content + + def test_minimal_skill_content_no_resources_element(self) -> None: + skill = _MinimalClassSkill() + assert "" not in skill.content + + def test_minimal_skill_content_no_scripts_element(self) -> None: + skill = _MinimalClassSkill() + assert "" not in skill.content + + def test_full_skill_has_resources(self) -> None: + skill = _FullClassSkill() + assert len(skill.resources) == 1 + assert skill.resources[0].name == "test-resource" + + def test_full_skill_has_scripts(self) -> None: + skill = _FullClassSkill() + assert len(skill.scripts) == 1 + assert skill.scripts[0].name == "test-script" + + def test_full_skill_content_contains_resources(self) -> None: + skill = _FullClassSkill() + assert "" in skill.content + assert 'name="test-resource"' in skill.content + + def test_full_skill_content_contains_scripts(self) -> None: + skill = _FullClassSkill() + assert "" in skill.content + assert 'name="test-script"' in skill.content + + def test_content_is_cached(self) -> None: + skill = _MinimalClassSkill() + content1 = skill.content + content2 = skill.content + assert content1 is content2 + + def test_resources_are_lazy_cached(self) -> None: + skill = _FullClassSkill() + resources1 = skill.resources + resources2 = skill.resources + assert resources1 is resources2 + + def test_scripts_are_lazy_cached(self) -> None: + skill = _FullClassSkill() + scripts1 = skill.scripts + scripts2 = skill.scripts + assert scripts1 is scripts2 + + def test_script_has_parameters_schema(self) -> None: + skill = _FullClassSkill() + script = skill.scripts[0] + assert isinstance(script, InlineSkillScript) + schema = script.parameters_schema + assert schema is not None + assert "value" in schema.get("properties", {}) + assert "factor" in schema.get("properties", {}) + + async def test_provider_with_class_skill(self) -> None: + skill = _FullClassSkill() + provider = SkillsProvider([skill]) + await _init_provider(provider) + + skills = _raw_skills(provider) + assert len(skills) == 1 + assert skills[0].name == "full-skill" + + async def test_provider_loads_class_skill_content(self) -> None: + skill = _FullClassSkill() + provider = SkillsProvider([skill]) + await _init_provider(provider) + + result = provider._load_skill(_raw_skills(provider), "full-skill") + assert "Use this skill for full tasks." in result + assert "" in result + assert "" in result + + async def test_in_memory_source_with_class_skill(self) -> None: + skill = _MinimalClassSkill() + source = InMemorySkillsSource([skill]) + skills = await source.get_skills() + assert len(skills) == 1 + assert skills[0].name == "minimal-skill" + + async def test_mixed_inline_and_class_skills(self) -> None: + inline = InlineSkill(name="inline-skill", description="Inline", instructions="inline body") + class_skill = _MinimalClassSkill() + provider = SkillsProvider([inline, class_skill]) + await _init_provider(provider) + + skills = _raw_skills(provider) + names = {s.name for s in skills} + assert names == {"inline-skill", "minimal-skill"} + + async def test_class_skill_script_runs(self) -> None: + skill = _FullClassSkill() + script = skill.scripts[0] + result = await script.run(skill, {"value": 10.0, "factor": 2.5}) + import json as _json + + parsed = _json.loads(result) + assert parsed["result"] == 25.0 + + async def test_class_skill_resource_reads(self) -> None: + skill = _FullClassSkill() + resource = skill.resources[0] + content = await resource.read() + assert content == "Static resource content." + + +# --------------------------------------------------------------------------- +# Tests: ClassSkill with decorator-based discovery +# --------------------------------------------------------------------------- + + +class _DecoratorClassSkill(ClassSkill): + """A class-based skill using @ClassSkill.resource and @ClassSkill.script decorators.""" + + def __init__(self) -> None: + super().__init__(name="decorator-skill", description="A decorator-discovered skill.") + + @property + def instructions(self) -> str: + return "Use this skill for decorator tests." + + @ClassSkill.resource(name="lookup-table") + def get_table(self) -> str: + """Conversion lookup table.""" + return "| From | To | Factor |" + + @ClassSkill.script(name="convert") + def run_convert(self, value: float, factor: float) -> str: + """Convert a value.""" + import json as _json + + return _json.dumps({"result": round(value * factor, 4)}) + + +class _BareDecoratorSkill(ClassSkill): + """Skill using bare decorators (no arguments) — name/description from method.""" + + def __init__(self) -> None: + super().__init__(name="bare-skill", description="Bare decorator skill.") + + @property + def instructions(self) -> str: + return "Bare instructions." + + @ClassSkill.resource + def my_table(self) -> str: + """The table docs.""" + return "table content" + + @ClassSkill.script + def my_script(self, x: int) -> int: + """Double x.""" + return x * 2 + + +class _DuplicateResourceSkill(ClassSkill): + """Skill with duplicate resource names — should raise.""" + + def __init__(self) -> None: + super().__init__(name="dup-skill", description="Dup.") + + @property + def instructions(self) -> str: + return "x" + + @ClassSkill.resource(name="same-name") + def res_a(self) -> str: + return "a" + + @ClassSkill.resource(name="same-name") + def res_b(self) -> str: + return "b" + + +class _DuplicateScriptSkill(ClassSkill): + """Skill with duplicate script names — should raise.""" + + def __init__(self) -> None: + super().__init__(name="dup-script-skill", description="Dup.") + + @property + def instructions(self) -> str: + return "x" + + @ClassSkill.script(name="same-name") + def script_a(self, x: int) -> int: + return x + + @ClassSkill.script(name="same-name") + def script_b(self, x: int) -> int: + return x + + +class _SelfAccessSkill(ClassSkill): + """Skill where resource/script access instance state via self.""" + + def __init__(self, multiplier: int = 10) -> None: + super().__init__(name="self-access", description="Self access skill.") + self.multiplier = multiplier + + @property + def instructions(self) -> str: + return "Use multiplier." + + @ClassSkill.resource(name="config") + def get_config(self) -> str: + return f"multiplier={self.multiplier}" + + @ClassSkill.script(name="multiply") + def multiply(self, value: int) -> int: + return value * self.multiplier + + +class TestClassSkillDecoratorDiscovery: + """Tests for decorator-based resource/script discovery on ClassSkill.""" + + def test_discovers_resources(self) -> None: + skill = _DecoratorClassSkill() + assert len(skill.resources) == 1 + assert skill.resources[0].name == "lookup-table" + + def test_discovers_scripts(self) -> None: + skill = _DecoratorClassSkill() + assert len(skill.scripts) == 1 + assert skill.scripts[0].name == "convert" + + def test_resource_description_from_decorator(self) -> None: + skill = _DecoratorClassSkill() + assert skill.resources[0].description is None + + def test_script_description_from_decorator(self) -> None: + skill = _DecoratorClassSkill() + assert skill.scripts[0].description is None + + def test_bare_decorator_name_from_method(self) -> None: + skill = _BareDecoratorSkill() + assert skill.resources[0].name == "my-table" + assert skill.scripts[0].name == "my-script" + + def test_bare_decorator_description_is_none(self) -> None: + skill = _BareDecoratorSkill() + assert skill.resources[0].description is None + assert skill.scripts[0].description is None + + async def test_resource_reads(self) -> None: + skill = _DecoratorClassSkill() + content = await skill.resources[0].read() + assert content == "| From | To | Factor |" + + async def test_script_runs(self) -> None: + skill = _DecoratorClassSkill() + import json as _json + + result = await skill.scripts[0].run(skill, {"value": 10.0, "factor": 2.5}) + parsed = _json.loads(result) + assert parsed["result"] == 25.0 + + def test_script_schema_excludes_self(self) -> None: + skill = _DecoratorClassSkill() + script = skill.scripts[0] + assert isinstance(script, InlineSkillScript) + schema = script.parameters_schema + assert schema is not None + props = schema.get("properties", {}) + assert "self" not in props + assert "value" in props + assert "factor" in props + + def test_resources_cached(self) -> None: + skill = _DecoratorClassSkill() + r1 = skill.resources + r2 = skill.resources + assert r1 == r2 + assert r1 is not r2 # defensive copy + + def test_scripts_cached(self) -> None: + skill = _DecoratorClassSkill() + s1 = skill.scripts + s2 = skill.scripts + assert s1 == s2 + assert s1 is not s2 # defensive copy + + def test_content_includes_discovered_resources(self) -> None: + skill = _DecoratorClassSkill() + assert "" in skill.content + assert 'name="lookup-table"' in skill.content + + def test_content_includes_discovered_scripts(self) -> None: + skill = _DecoratorClassSkill() + assert "" in skill.content + assert 'name="convert"' in skill.content + + def test_duplicate_resource_name_raises(self) -> None: + skill = _DuplicateResourceSkill() + with pytest.raises(ValueError, match="already has a resource named"): + _ = skill.resources + + def test_duplicate_script_name_raises(self) -> None: + skill = _DuplicateScriptSkill() + with pytest.raises(ValueError, match="already has a script named"): + _ = skill.scripts + + async def test_self_access_resource(self) -> None: + skill = _SelfAccessSkill(multiplier=42) + content = await skill.resources[0].read() + assert content == "multiplier=42" + + async def test_self_access_script(self) -> None: + skill = _SelfAccessSkill(multiplier=3) + result = await skill.scripts[0].run(skill, {"value": 7}) + assert result == 21 + + def test_no_decorators_yields_empty(self) -> None: + skill = _MinimalClassSkill() + assert skill.resources == [] + assert skill.scripts == [] + + async def test_provider_with_decorator_skill(self) -> None: + skill = _DecoratorClassSkill() + provider = SkillsProvider([skill]) + await _init_provider(provider) + + skills = _raw_skills(provider) + assert len(skills) == 1 + assert skills[0].name == "decorator-skill" + + def test_manual_override_wins(self) -> None: + """A subclass that overrides resources/scripts bypasses decorator discovery.""" + skill = _FullClassSkill() + assert len(skill.resources) == 1 + assert skill.resources[0].name == "test-resource" + + async def test_property_resource_reads(self) -> None: + """@ClassSkill.resource on a @property works correctly.""" + skill = _PropertyResourceSkill() + assert len(skill.resources) == 1 + assert skill.resources[0].name == "static-table" + content = await skill.resources[0].read() + assert "miles" in content + + def test_property_resource_description_is_none_without_explicit(self) -> None: + skill = _PropertyResourceSkill() + assert skill.resources[0].description is None + + def test_property_resource_in_content(self) -> None: + skill = _PropertyResourceSkill() + assert 'name="static-table"' in skill.content + + async def test_mixed_property_and_method_resources(self) -> None: + """Property and method resources can coexist.""" + skill = _MixedPropertyMethodSkill() + names = {r.name for r in skill.resources} + assert names == {"prop-data", "method-data"} + for r in skill.resources: + content = await r.read() + assert "content" in content.lower() + + def test_explicit_resource_description_in_object(self) -> None: + """Explicit description= on @ClassSkill.resource is stored on the object.""" + skill = _ExplicitDescriptionSkill() + res = next(r for r in skill.resources if r.name == "described-res") + assert res.description == "A described resource." + + def test_explicit_script_description_in_object(self) -> None: + """Explicit description= on @ClassSkill.script is stored on the object.""" + skill = _ExplicitDescriptionSkill() + scr = next(s for s in skill.scripts if s.name == "described-scr") + assert scr.description == "A described script." + + def test_explicit_description_in_content_xml(self) -> None: + """Explicit descriptions appear in the skill content XML.""" + skill = _ExplicitDescriptionSkill() + assert 'description="A described resource."' in skill.content + assert 'description="A described script."' in skill.content + + def test_property_getter_not_called_during_discovery(self) -> None: + """Property getter must NOT be evaluated when resources are discovered.""" + skill = _PropertyCallCountSkill() + assert skill.getter_call_count == 0 + _ = skill.resources # discovery should NOT call the getter + assert skill.getter_call_count == 0 + + async def test_property_getter_called_on_read(self) -> None: + """Property getter IS evaluated when the resource is read.""" + skill = _PropertyCallCountSkill() + _ = skill.resources + assert skill.getter_call_count == 0 + await skill.resources[0].read() + assert skill.getter_call_count == 1 + + def test_make_method_name_strips_leading_trailing_hyphens(self) -> None: + """_make_method_name strips leading/trailing underscores turned to hyphens.""" + from agent_framework._skills import _make_method_name + + assert _make_method_name("my_method") == "my-method" + assert _make_method_name("_private_method_") == "private-method" + assert _make_method_name("__dunder__") == "dunder" + assert _make_method_name("already_good") == "already-good" + + def test_inherited_decorated_resources_are_discovered(self) -> None: + """Decorated resources from a parent class are discovered on subclass.""" + skill = _ChildSkill() + names = {r.name for r in skill.resources} + assert "parent-data" in names + + def test_inherited_decorated_scripts_are_discovered(self) -> None: + """Decorated scripts from a parent class are discovered on subclass.""" + skill = _ChildSkill() + names = {s.name for s in skill.scripts} + assert "parent-action" in names + + def test_child_can_add_own_resources(self) -> None: + """A child class can add resources alongside inherited ones.""" + skill = _ChildSkill() + names = {r.name for r in skill.resources} + assert "parent-data" in names + assert "child-data" in names + + async def test_script_receives_kwargs(self) -> None: + """ClassSkill scripts receive **kwargs forwarded from the runtime.""" + skill = _KwargsSkill() + script = skill.scripts[0] + result = await script.run(skill, {"x": 5}, custom_key="hello") + assert result == "5-hello" + + def test_wrong_decorator_order_resource_raises(self) -> None: + """@ClassSkill.resource above @property raises TypeError at class definition.""" + with pytest.raises(TypeError, match="must be applied before @property"): + + class _BadOrder(ClassSkill): + def __init__(self) -> None: + super().__init__(name="bad", description="bad") + + @property + def instructions(self) -> str: + return "x" + + @ClassSkill.resource(name="oops") # wrong: should be below @property + @property + def bad_prop(self) -> str: + return "x" + + def test_wrong_decorator_order_script_raises(self) -> None: + """@ClassSkill.script on a property raises TypeError.""" + with pytest.raises(TypeError, match="must be applied before"): + + class _BadOrder(ClassSkill): + def __init__(self) -> None: + super().__init__(name="bad", description="bad") + + @property + def instructions(self) -> str: + return "x" + + @ClassSkill.script(name="oops") + @property + def bad_prop(self) -> str: + return "x" + + def test_invalid_explicit_resource_name_raises(self) -> None: + """Invalid name= on @ClassSkill.resource raises ValueError at decoration.""" + with pytest.raises(ValueError, match="Invalid @ClassSkill.resource name"): + + class _BadName(ClassSkill): + def __init__(self) -> None: + super().__init__(name="bad", description="bad") + + @property + def instructions(self) -> str: + return "x" + + @ClassSkill.resource(name="UPPER CASE!") + def res(self) -> str: + return "x" + + def test_invalid_explicit_script_name_raises(self) -> None: + """Invalid name= on @ClassSkill.script raises ValueError at decoration.""" + with pytest.raises(ValueError, match="Invalid @ClassSkill.script name"): + + class _BadName(ClassSkill): + def __init__(self) -> None: + super().__init__(name="bad", description="bad") + + @property + def instructions(self) -> str: + return "x" + + @ClassSkill.script(name="has spaces") + def scr(self, x: int) -> int: + return x + + def test_empty_explicit_name_raises(self) -> None: + """Empty name= on @ClassSkill.resource raises ValueError.""" + with pytest.raises(ValueError, match="name cannot be empty"): + + class _EmptyName(ClassSkill): + def __init__(self) -> None: + super().__init__(name="bad", description="bad") + + @property + def instructions(self) -> str: + return "x" + + @ClassSkill.resource(name="") + def res(self) -> str: + return "x" + + def test_resources_copy_prevents_cache_mutation(self) -> None: + """Mutating the returned resources list does not affect the cache.""" + skill = _DecoratorClassSkill() + r1 = skill.resources + r1.clear() + r2 = skill.resources + assert len(r2) == 1 # original cached list is intact + + def test_scripts_copy_prevents_cache_mutation(self) -> None: + """Mutating the returned scripts list does not affect the cache.""" + skill = _DecoratorClassSkill() + s1 = skill.scripts + s1.clear() + s2 = skill.scripts + assert len(s2) == 1 # original cached list is intact + + async def test_inherited_property_resource_discovered(self) -> None: + """A @property @ClassSkill.resource on a parent class is discovered on child.""" + skill = _ChildWithInheritedPropertySkill() + names = {r.name for r in skill.resources} + assert "parent-prop" in names + content = await next(r for r in skill.resources if r.name == "parent-prop").read() + assert content == "parent property content" + + +# --------------------------------------------------------------------------- +# Helper skills for additional tests +# --------------------------------------------------------------------------- + + +class _ExplicitDescriptionSkill(ClassSkill): + """Skill with explicit descriptions on decorator.""" + + def __init__(self) -> None: + super().__init__(name="desc-skill", description="Explicit desc.") + + @property + def instructions(self) -> str: + return "x" + + @ClassSkill.resource(name="described-res", description="A described resource.") + def res(self) -> str: + return "data" + + @ClassSkill.script(name="described-scr", description="A described script.") + def scr(self, x: int) -> int: + return x + + +class _PropertyCallCountSkill(ClassSkill): + """Tracks how many times the property getter is called.""" + + def __init__(self) -> None: + super().__init__(name="callcount-skill", description="Tracks calls.") + self.getter_call_count = 0 + + @property + def instructions(self) -> str: + return "x" + + @property + @ClassSkill.resource(name="counted") + def counted_resource(self) -> str: + self.getter_call_count += 1 + return "counted" + + +class _ParentSkill(ClassSkill, ABC): + """Parent with decorated resources/scripts.""" + + @ClassSkill.resource(name="parent-data") + def parent_resource(self) -> str: + return "parent" + + @ClassSkill.script(name="parent-action") + def parent_script(self, x: int) -> int: + return x + + +class _ChildSkill(_ParentSkill): + """Child inheriting parent resources and adding its own.""" + + def __init__(self) -> None: + super().__init__(name="child-skill", description="Child.") + + @property + def instructions(self) -> str: + return "child" + + @ClassSkill.resource(name="child-data") + def child_resource(self) -> str: + return "child" + + +class _KwargsSkill(ClassSkill): + """Skill that uses **kwargs from runtime.""" + + def __init__(self) -> None: + super().__init__(name="kwargs-skill", description="Kwargs.") + + @property + def instructions(self) -> str: + return "x" + + @ClassSkill.script(name="echo") + def echo(self, x: int, **kwargs: Any) -> str: + return f"{x}-{kwargs.get('custom_key', 'none')}" + + +class _ParentWithPropertyResource(ClassSkill, ABC): + """Parent with a property-based resource.""" + + @property + @ClassSkill.resource(name="parent-prop") + def parent_property(self) -> str: + return "parent property content" + + +class _ChildWithInheritedPropertySkill(_ParentWithPropertyResource): + """Child that should discover inherited property resource.""" + + def __init__(self) -> None: + super().__init__(name="child-prop-skill", description="Child prop.") + + @property + def instructions(self) -> str: + return "x" + + +class _PropertyResourceSkill(ClassSkill): + """Skill with a property-based resource.""" + + def __init__(self) -> None: + super().__init__(name="prop-skill", description="Property skill.") + + @property + def instructions(self) -> str: + return "Use this skill." + + @property + @ClassSkill.resource(name="static-table") + def conversion_table(self) -> str: + """Static conversion table.""" + return "| miles | km | 1.60934 |" + + +class _MixedPropertyMethodSkill(ClassSkill): + """Skill with both property and method resources.""" + + def __init__(self) -> None: + super().__init__(name="mixed-prop", description="Mixed.") + + @property + def instructions(self) -> str: + return "x" + + @property + @ClassSkill.resource(name="prop-data") + def static_data(self) -> str: + """Static content.""" + return "Property Content" + + @ClassSkill.resource(name="method-data") + def dynamic_data(self) -> str: + """Dynamic content.""" + return "Method Content" + async def test_code_skill_scripts_element_contains_parameters(self) -> None: """Scripts XML includes parameters schema when the function has typed parameters.""" diff --git a/python/samples/02-agents/skills/README.md b/python/samples/02-agents/skills/README.md index b401278577..6e9bb08202 100644 --- a/python/samples/02-agents/skills/README.md +++ b/python/samples/02-agents/skills/README.md @@ -10,7 +10,8 @@ Start with file-based or code-defined skills, then explore combining them and ad |--------|-------------| | [**file_based_skill**](file_based_skill/) | Define skills as `SKILL.md` files on disk with reference documents and executable scripts. Uses the unit-converter skill. | | [**code_defined_skill**](code_defined_skill/) | Define skills entirely in Python code using `Skill`, `@skill.resource`, and `@skill.script` decorators. Uses a code-defined unit-converter skill. | -| [**mixed_skills**](mixed_skills/) | Combine code-defined and file-based skills in a single agent. Uses a code-defined volume-converter and a file-based unit-converter. | +| [**class_based_skill**](class_based_skill/) | Define skills as Python classes using `ClassSkill` with `@ClassSkill.resource` and `@ClassSkill.script` decorators for auto-discovery. Uses a class-based unit-converter skill. | +| [**mixed_skills**](mixed_skills/) | Combine code-defined, class-based, and file-based skills in a single agent. Uses a code-defined volume-converter, a class-based temperature-converter, and a file-based unit-converter. | | [**script_approval**](script_approval/) | Require human-in-the-loop approval before executing skill scripts | ## Key Concepts @@ -23,17 +24,18 @@ Skills use a three-step interaction model to minimize token usage: 2. **Load** — Full instructions are loaded on-demand via the `load_skill` tool 3. **Access** — Resources are read via `read_skill_resource`; scripts are executed via `run_skill_script` -### File-Based vs Code-Defined Skills +### File-Based vs Code-Defined vs Class-Based Skills -| Aspect | File-Based | Code-Defined | -|--------|-----------|--------------| -| Definition | `SKILL.md` files on disk | `Skill` instances in Python | -| Resources | Static files in `references/` and `assets/` directories | Callable functions via `@skill.resource` decorator | -| Scripts | Python files in `scripts/` directory (executed via subprocess) | Callable functions via `@skill.script` decorator (executed in-process) | -| Discovery | Automatic via `skill_paths` parameter | Explicit via `skills` parameter | -| Dynamic content | No (static files only) | Yes (functions can generate content at runtime) | +| Aspect | File-Based | Code-Defined | Class-Based | +|--------|-----------|--------------|-------------| +| Definition | `SKILL.md` files on disk | `Skill` instances in Python | Classes extending `ClassSkill` | +| Resources | Static files in `references/` and `assets/` directories | Callable functions via `@skill.resource` decorator | `@ClassSkill.resource` decorator (auto-discovered) | +| Scripts | Python files in `scripts/` directory (executed via subprocess) | Callable functions via `@skill.script` decorator (executed in-process) | `@ClassSkill.script` decorator (executed in-process) | +| Discovery | Automatic via `skill_paths` parameter | Explicit via `skills` parameter | Explicit via `skills` parameter | +| Dynamic content | No (static files only) | Yes (functions can generate content at runtime) | Yes (functions can generate content at runtime) | +| Sharing pattern | Copy skill directory | Inline or shared instances | Package in shared libraries/PyPI | -Both types can be combined in a single `SkillsProvider` — see the [mixed_skills](mixed_skills/) sample. +All three types can be combined in a single `SkillsProvider` — see the [mixed_skills](mixed_skills/) sample. ### Script Execution diff --git a/python/samples/02-agents/skills/class_based_skill/README.md b/python/samples/02-agents/skills/class_based_skill/README.md new file mode 100644 index 0000000000..bf70e35db8 --- /dev/null +++ b/python/samples/02-agents/skills/class_based_skill/README.md @@ -0,0 +1,71 @@ +# Class-Based Agent Skills + +This sample demonstrates how to define **Agent Skills as Python classes** using `ClassSkill`. + +## What's Demonstrated + +- Creating skills as classes that extend `ClassSkill` +- Bundling name, description, instructions, resources, and scripts into a single class +- Using `@ClassSkill.resource` decorator for automatic resource discovery +- Using `@ClassSkill.script` decorator for automatic script discovery +- Lazy-loading and caching of resources and scripts +- Registering class-based skills with `SkillsProvider` + +## Skills Included + +### unit-converter (class-based) + +A `UnitConverterSkill` class that converts between common units. Defined in `class_based_skill.py`: + +- `conversion-table` — Static resource with factor table +- `convert` — Script that performs `value × factor` conversion + +## Project Structure + +``` +class_based_skill/ +├── class_based_skill.py +└── README.md +``` + +## Running the Sample + +### Prerequisites + +- An [Azure AI Foundry](https://ai.azure.com/) project with a deployed model (e.g. `gpt-4o-mini`) + +### Environment Variables + +Set the required environment variables in a `.env` file (see `python/.env.example`): + +- `FOUNDRY_PROJECT_ENDPOINT`: Your Azure AI Foundry project endpoint +- `FOUNDRY_MODEL`: The name of your model deployment (defaults to `gpt-4o-mini`) + +### Authentication + +This sample uses `AzureCliCredential` for authentication. Run `az login` in your terminal before running the sample. + +### Run + +```bash +cd python +uv run samples/02-agents/skills/class_based_skill/class_based_skill.py +``` + +### Expected Output + +``` +Converting units with class-based skills +------------------------------------------------------------ +Agent: Here are your conversions: + +1. **26.2 miles → 42.16 km** (a marathon distance) +2. **75 kg → 165.35 lbs** +``` + +## Learn More + +- [Agent Skills Specification](https://agentskills.io/) +- [Code-Defined Skills Sample](../code_defined_skill/) +- [Mixed Skills Sample](../mixed_skills/) +- [Microsoft Agent Framework Documentation](../../../../../docs/) diff --git a/python/samples/02-agents/skills/class_based_skill/class_based_skill.py b/python/samples/02-agents/skills/class_based_skill/class_based_skill.py new file mode 100644 index 0000000000..e2b480864f --- /dev/null +++ b/python/samples/02-agents/skills/class_based_skill/class_based_skill.py @@ -0,0 +1,145 @@ +# Copyright (c) Microsoft. All rights reserved. + +import asyncio +import json +import os + +# Uncomment this filter to suppress the experimental Skills warning before +# using the sample's Skills APIs. +# import warnings # isort: skip +# warnings.filterwarnings("ignore", message=r"\[SKILLS\].*", category=FutureWarning) +from textwrap import dedent + +from agent_framework import Agent, ClassSkill, SkillsProvider +from agent_framework.foundry import FoundryChatClient +from azure.identity import AzureCliCredential +from dotenv import load_dotenv + +""" +Class-Based Agent Skills — Define skills as Python classes + +This sample demonstrates how to define Agent Skills as reusable Python classes +by subclassing ``ClassSkill``. Class-based skills bundle all components (name, +description, instructions, resources, scripts) into a single class, making +them easy to package and distribute via shared libraries or PyPI. + +Key concepts shown: +- Subclassing ``ClassSkill`` to create a self-contained skill +- Using ``@property`` + ``@ClassSkill.resource`` (bare) — name defaults to method name +- Using ``@ClassSkill.script(name=..., description=...)`` — explicit name and description +- Lazy-loading and caching of resources and scripts +""" + +# Load environment variables from .env file +load_dotenv() + + +# --------------------------------------------------------------------------- +# Class-Based Skill: UnitConverterSkill +# --------------------------------------------------------------------------- + + +class UnitConverterSkill(ClassSkill): + """A unit-converter skill defined as a Python class. + + Converts between common units (miles↔km, pounds↔kg) using a + conversion factor. Resources and scripts are discovered automatically + via decorators. + """ + + def __init__(self) -> None: + super().__init__( + name="unit-converter", + description=( + "Convert between common units using a multiplication factor. " + "Use when asked to convert miles, kilometers, pounds, or kilograms." + ), + ) + + @property + def instructions(self) -> str: + return dedent("""\ + Use this skill when the user asks to convert between units. + + 1. Review the conversion-table resource to find the factor for the requested conversion. + 2. Use the convert script, passing the value and factor from the table. + 3. Present the result clearly with both units. + """) + + # 1. Property with bare decorator — name defaults to the method name + # ("conversion_table" → "conversion-table"), no description. + # Place @property first, then @ClassSkill.resource. + @property + @ClassSkill.resource + def conversion_table(self) -> str: + """Lookup table of multiplication factors for common unit conversions.""" + return dedent("""\ + # Conversion Tables + + Formula: **result = value × factor** + + | From | To | Factor | + |-------------|-------------|----------| + | miles | kilometers | 1.60934 | + | kilometers | miles | 0.621371 | + | pounds | kilograms | 0.453592 | + | kilograms | pounds | 2.20462 | + """) + + # 2. Explicit name — overrides the method name + # 3. Explicit description — provides a description for the script + @ClassSkill.script(name="convert", description="Multiplies a value by a conversion factor.") + def convert_units(self, value: float, factor: float) -> str: + """Convert a value using a multiplication factor: result = value × factor. + + Args: + value: The numeric value to convert. + factor: Conversion factor from the conversion table. + + Returns: + JSON string with the inputs and converted result. + """ + result = round(value * factor, 4) + return json.dumps({"value": value, "factor": factor, "result": result}) + + +async def main() -> None: + """Run the class-based skills demo.""" + endpoint = os.environ["FOUNDRY_PROJECT_ENDPOINT"] + deployment = os.environ.get("FOUNDRY_MODEL", "gpt-4o-mini") + + client = FoundryChatClient( + project_endpoint=endpoint, + model=deployment, + credential=AzureCliCredential(), + ) + + # Instantiate the class-based skill and pass it to the provider + unit_converter = UnitConverterSkill() + + async with Agent( + client=client, + instructions="You are a helpful assistant that can convert units.", + context_providers=[SkillsProvider(unit_converter)], + ) as agent: + print("Converting units with class-based skills") + print("-" * 60) + response = await agent.run( + "How many kilometers is a marathon (26.2 miles)? And how many pounds is 75 kilograms?" + ) + print(f"Agent: {response}\n") + + +if __name__ == "__main__": + asyncio.run(main()) + +""" +Sample output: + +Converting units with class-based skills +------------------------------------------------------------ +Agent: Here are your conversions: + +1. **26.2 miles → 42.16 km** (a marathon distance) +2. **75 kg → 165.35 lbs** +""" diff --git a/python/samples/02-agents/skills/mixed_skills/README.md b/python/samples/02-agents/skills/mixed_skills/README.md index 8e5111266c..3b6832827e 100644 --- a/python/samples/02-agents/skills/mixed_skills/README.md +++ b/python/samples/02-agents/skills/mixed_skills/README.md @@ -1,17 +1,18 @@ -# Mixed Skills — Code Skills and File Skills +# Mixed Skills — Code, Class, and File Skills -This sample demonstrates how to combine **code-defined skills** and -**file-based skills** in a single agent using a `SkillScriptRunner` callable -and `SkillsProvider`. +This sample demonstrates how to combine **code-defined skills**, +**class-based skills**, and **file-based skills** in a single agent using +`SkillsProvider`. ## Concepts | Concept | Description | |---------|-------------| | **Code skill** | A `Skill` created in Python with `@skill.script` decorators for in-process callable functions and `@skill.resource` for dynamic content | +| **Class skill** | A self-contained skill class extending `ClassSkill`, bundling instructions, resources, and scripts | | **File skill** | A skill discovered from a `SKILL.md` file on disk, with reference documents and executable script files | | **`script_runner`** | A callable (sync or async) satisfying the `SkillScriptRunner` protocol — required when file skills have scripts | -| **`SkillsProvider`** | Registers both code-defined and file-based skills in a single provider | +| **`SkillsProvider`** | Registers code-defined, class-based, and file-based skills in a single provider | ## Skills in This Sample @@ -24,6 +25,15 @@ Defined entirely in Python code using decorators: Code scripts run **in-process** — no subprocess or external runner needed. +### temperature-converter (class skill) + +Defined as a `TemperatureConverterSkill` class extending `ClassSkill`: + +- **`@ClassSkill.resource`** — `temperature-conversion-formulas`: °F↔°C↔K formulas +- **`@ClassSkill.script`** — `convert-temperature`: converts between temperature scales + +Class-based scripts run **in-process** — no subprocess or external runner needed. + ### unit-converter (file skill) Discovered from `skills/unit-converter/SKILL.md`: @@ -43,7 +53,10 @@ File scripts are executed as **local Python subprocesses** via the │ AggregatingSkillsSource([ │ │ FileSkillsSource("./skills", # file skills │ │ script_runner=runner), │ -│ InMemorySkillsSource([skill]), # code skills │ +│ InMemorySkillsSource([ │ +│ volume_skill, # code skill │ +│ temp_converter, # class skill │ +│ ]), │ │ ]) │ │ ) │ │ ) │ @@ -54,6 +67,7 @@ File scripts are executed as **local Python subprocesses** via the │ script_runner(skill, script, args) │ │ │ │ • Code scripts (@skill.script) → in-process call │ +│ • Class scripts (@ClassSkill.script) → in-process call │ │ • File scripts (scripts/*.py) → subprocess via │ │ the callback function │ └─────────────────────────────────────────────────────────────┘ diff --git a/python/samples/02-agents/skills/mixed_skills/mixed_skills.py b/python/samples/02-agents/skills/mixed_skills/mixed_skills.py index 998dc6e692..2b10fc0c2a 100644 --- a/python/samples/02-agents/skills/mixed_skills/mixed_skills.py +++ b/python/samples/02-agents/skills/mixed_skills/mixed_skills.py @@ -16,6 +16,7 @@ from typing import Any from agent_framework import ( Agent, AggregatingSkillsSource, + ClassSkill, DeduplicatingSkillsSource, FileSkillsSource, InlineSkill, @@ -34,28 +35,32 @@ if _SKILLS_ROOT not in sys.path: from subprocess_script_runner import subprocess_script_runner # noqa: E402 """ -Mixed Skills — Code skills and file skills in a single agent +Mixed Skills — Code, class, and file skills in a single agent This sample demonstrates how to combine **code-defined skills** (with -``@skill.script`` and ``@skill.resource`` decorators) and **file-based skills** -(discovered from ``SKILL.md`` files on disk) in a single agent using -``SkillsProvider`` and a ``SkillScriptRunner`` callable. +``@skill.script`` and ``@skill.resource`` decorators), **class-based skills** +(subclassing ``ClassSkill``), and **file-based skills** (discovered from +``SKILL.md`` files on disk) in a single agent using ``SkillsProvider`` and +a ``SkillScriptRunner`` callable. Key concepts shown: - Code skills with ``@skill.script``: executable Python functions the agent can invoke directly in-process. - Code skills with ``@skill.resource``: dynamic content the agent can read on demand. +- Class skills: self-contained skill classes extending ``ClassSkill``. - File skills from disk: ``SKILL.md`` files with reference documents and executable script files. - ``script_runner``: routes **file-based** script execution through a callback, enabling custom handling (e.g. subprocess calls). - Code-defined scripts (``@skill.script``) run in-process automatically. + Code-defined and class-based scripts run in-process automatically. -The sample registers two skills: +The sample registers three skills: 1. **volume-converter** (code skill) — converts between gallons and liters using ``@skill.script`` for conversion and ``@skill.resource`` for the factor table. -2. **unit-converter** (file skill) — converts between common units (miles↔km, +2. **temperature-converter** (class skill) — converts between temperature scales + (°F↔°C↔K) using a ``ClassSkill`` subclass. +3. **unit-converter** (file skill) — converts between common units (miles↔km, pounds↔kg) via a subprocess-executed Python script discovered from ``skills/unit-converter/SKILL.md``. """ @@ -110,9 +115,68 @@ def convert_volume(value: float, factor: float) -> str: # --------------------------------------------------------------------------- -# 2. Wire everything together and run the agent +# 2. Define a class-based skill for temperature conversion # --------------------------------------------------------------------------- +class TemperatureConverterSkill(ClassSkill): + """A temperature-converter skill defined as a Python class. + + Converts between temperature scales (Fahrenheit, Celsius, Kelvin). + Resources and scripts are discovered automatically via decorators. + """ + + def __init__(self) -> None: + super().__init__( + name="temperature-converter", + description="Convert between temperature scales (Fahrenheit, Celsius, Kelvin).", + ) + + @property + def instructions(self) -> str: + return dedent("""\ + Use this skill when the user asks to convert temperatures. + + 1. Read the temperature-conversion-formulas resource to find the factor and offset + for the requested conversion. + 2. Use the convert-temperature script, passing value, factor, and offset. + 3. Present the result clearly with both temperature scales. + """) + + @ClassSkill.resource(name="temperature-conversion-formulas") + def formulas(self) -> str: + """Temperature conversion formulas reference table.""" + return dedent("""\ + # Temperature Conversion Formulas + + Formula: **result = value × factor + offset** + + | From | To | Factor | Offset | + |-------------|-------------|----------|-----------| + | Fahrenheit | Celsius | 0.555556 | -17.7778 | + | Celsius | Fahrenheit | 1.8 | 32 | + | Celsius | Kelvin | 1 | 273.15 | + | Kelvin | Celsius | 1 | -273.15 | + """) + + @ClassSkill.script(name="convert-temperature") + def convert_temperature(self, value: float, factor: float, offset: float = 0) -> str: + """Convert a temperature value using factor and offset from the formulas resource. + + Args: + value: The numeric temperature value to convert. + factor: Conversion factor from the formulas resource. + offset: Offset to add after multiplying (default 0). + + Returns: + JSON string with the conversion result. + """ + result = round(value * factor + offset, 4) + return json.dumps({"value": value, "factor": factor, "offset": offset, "result": result}) + + +# --------------------------------------------------------------------------- +# 3. Wire everything together and run the agent +# --------------------------------------------------------------------------- async def main() -> None: """Run the combined skills demo.""" @@ -126,9 +190,11 @@ async def main() -> None: credential=AzureCliCredential(), ) - # Create the SkillsProvider with both code and file skills. - # The script_runner handles file-based scripts; code-defined scripts - # (@skill.script) run in-process automatically. + # Create the SkillsProvider with code, class, and file skills. + # The script_runner handles file-based scripts; code-defined and + # class-based scripts run in-process automatically. + temperature_converter = TemperatureConverterSkill() + skills_provider = SkillsProvider( DeduplicatingSkillsSource( AggregatingSkillsSource([ @@ -136,7 +202,7 @@ async def main() -> None: str(Path(__file__).parent / "skills"), script_runner=subprocess_script_runner, ), - InMemorySkillsSource([volume_converter_skill]), + InMemorySkillsSource([volume_converter_skill, temperature_converter]), ]) ) ) @@ -144,14 +210,17 @@ async def main() -> None: # Run the agent async with Agent( client=client, - instructions="You are a helpful assistant that can convert units.", + instructions="You are a helpful assistant that can convert units, volumes, and temperatures.", context_providers=[skills_provider], ) as agent: - # Ask the agent to use both skills - print("Converting units") + # Ask the agent to use all three skills + print("Converting with mixed skills (file + code + class)") print("-" * 60) response = await agent.run( - "How many kilometers is a marathon (26.2 miles)? And how many liters is a 5-gallon bucket?" + "I need three conversions: " + "1) How many kilometers is a marathon (26.2 miles)? " + "2) How many liters is a 5-gallon bucket? " + "3) What is 98.6°F in Celsius?" ) print(f"Agent: {response}\n") @@ -162,12 +231,11 @@ if __name__ == "__main__": """ Sample output: -Converting units +Converting with mixed skills (file + code + class) ------------------------------------------------------------ Agent: Here are your conversions: 1. **26.2 miles → 42.16 km** (a marathon distance) 2. **5 gallons → 18.93 liters** - -I used the conversion factors from each skill's reference table. +3. **98.6°F → 37.0°C** """ From c06af9a1b3db2f55b56a51c53b0f9be840b56737 Mon Sep 17 00:00:00 2001 From: Giles Odigwe <79032838+giles17@users.noreply.github.com> Date: Thu, 7 May 2026 13:39:32 -0700 Subject: [PATCH 4/6] .NET: Python: Add dotnet integration test report to CI (#5515) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add dotnet integration test report to CI - Add --report-junit flag to dotnet integration test step to generate JUnit XML alongside TRX, with explicit --results-directory to centralize output in IntegrationTestResults/ - Upload JUnit XML artifacts from each matrix leg (net10.0/ubuntu, net472/windows) as dotnet-test-results-{framework}-{os} - Add dotnet-integration-test-report job that downloads artifacts, runs the existing aggregate.py script, posts markdown to Job Summary, and saves trend history via actions/cache - Refactor aggregate.py to discover JUnit XML files recursively, supporting both pytest (pytest.xml) and xunit (*.junit.xml) layouts - Handle provider name derivation for dotnet artifact naming convention - Fix nodeid collision when same test runs under multiple frameworks by qualifying keys with provider when collisions are detected - Improve module extraction for dotnet C# classnames (recognizes IntegrationTests/UnitTests namespace segments) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: trigger dotnet CI for report validation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: use .junit extension (not .junit.xml) for xunit v3 output xUnit v3 generates files with .junit extension, not .junit.xml. Update upload glob and aggregate.py discovery to match. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: use deterministic provider-qualified keys for dotnet tests Always prefix dotnet test keys with provider (e.g. net10.0 (ubuntu)::TestName) to ensure stable, comparable counts across runs regardless of file parse order. Also show Executed (passed+failed) instead of Total in summary table. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: match Python report summary format (Total, passed/total, etc.) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat: split dotnet report into per-framework tables Dotnet tests run on multiple frameworks (net10.0, net472). Instead of one combined table with unstable totals, show separate sections per framework — each with its own summary row and per-test table. Python reports retain the original single-table format. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Re-enable 7 flaky dotnet integration tests with increased timeouts Increase timeouts to reduce timing-related flakiness in LLM-backed integration tests (issue #4971): - ExternalClientTests: 60s -> 120s default timeout - SamplesValidationBase: 60s -> 120s default timeout - ConsoleAppSamplesValidation: 90s -> 150s for long-running tests - AzureFunctions SamplesValidation: 2min -> 3min orchestration timeout, 60s -> 90s per-step WaitForConditionAsync timeouts Remove all Skip=Flaky annotations and unused SkipFlakyTimingTest constants. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Re-skip LLM non-determinism flaky tests, keep timeout fixes Re-skip SingleAgentOrchestrationHITLSampleValidationAsync and LongRunningToolsSampleValidationAsync - these fail due to LLM producing extra review notifications, not timeouts. Updated skip reasons to accurately describe the root cause. Reverted unnecessary timeout change on the skipped LongRunningTools test. The remaining 5 re-enabled tests with timeout increases are stable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Enable Anthropic integration tests in CI Replace hardcoded skip with conditional skip pattern (matching CopilotStudio approach): tests gracefully skip when ANTHROPIC_API_KEY is missing, and run when present. Changes: - AnthropicChatCompletionFixture: try/catch in InitializeAsync with Assert.Skip on missing config (replaces hardcoded SkipReason) - AnthropicSkillsIntegrationTests: same pattern per test method - dotnet-build-and-test.yml: wire up ANTHROPIC_API_KEY, ANTHROPIC_CHAT_MODEL_NAME, and ANTHROPIC_REASONING_MODEL_NAME env vars to the integration test step Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix missing System using in AnthropicSkillsIntegrationTests Add 'using System;' for InvalidOperationException in try/catch blocks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Skip flaky SingleAgentOrchestrationChainingSampleValidationAsync LLM non-determinism causes Assert.NotNull failures on orchestration results. Skip until test logic is hardened against non-deterministic LLM responses. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Re-enable HITL and LongRunningTools tests with timeout and flexibility fixes - Remove Skip attribute from SingleAgentOrchestrationHITLSampleValidationAsync - Remove Skip attribute from LongRunningToolsSampleValidationAsync - Increase timeout from 120s/90s to 180s to accommodate 2+ LLM round-trips - Replace rigid 2-cycle assertion with flexible approval logic that handles extra review cycles from LLM non-determinism Fixes the two failure modes identified in #4971: 1. Timeout: 120s/90s was insufficient for multiple LLM calls under CI load 2. Extra notifications: Assert.Fail on 3rd+ review cycle was too rigid Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Increase AzureFunctions LongRunningTools test timeouts from 90s to 180s The LongRunningToolsSampleValidationAsync test in the AzureFunctions integration tests was failing in CI with TimeoutException at the 'Content published notification is logged' step. The 90-second timeouts are too tight for CI environments where LLM calls and orchestration overhead can be slow. Increased all three WaitForConditionAsync timeouts from 90s to 180s: - Waiting for human feedback notification - Waiting for publish notification (the step that was failing) - Waiting for orchestration completion Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Merge main and fix dotnet report path after flaky_report rename Merge upstream/main which renamed scripts/flaky_report/ to scripts/integration_test_report/ (from Python PR #5454). Update the dotnet-build-and-test workflow to reference the new path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add RetryFact to DurableTask and AzureFunctions integration tests These tests interact with LLMs via stdin/stdout (DurableTask) or HTTP (AzureFunctions) and are inherently non-deterministic. Unlike the Python side which uses pytest-retry, the dotnet tests had no retry mechanism and a single transient failure would fail the entire CI run. Changes: - Switch [Fact] to [RetryFact(2, 5000)] on all LLM-dependent tests across ConsoleAppSamplesValidation, ExternalClientTests, WorkflowConsoleAppSamplesValidation, and AzureFunctions SamplesValidation - Add re-prompt mechanism to LongRunningToolsSampleValidationAsync: if the LLM doesn't invoke the tool within 60s, re-send the prompt (up to 2 retries) instead of burning the full timeout - Reduce LongRunningTools timeout from 240s to 180s (re-prompt makes the extra buffer unnecessary) - Leave simple/deterministic tests as [Fact] (SingleAgent, unit tests) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add persist-credentials: false to Integration Test Report checkout step Matches the convention used by other checkout steps in this workflow to avoid leaving GITHUB_TOKEN credentials in the local git config. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * small fixes * disable anthropic failing tests --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/dotnet-build-and-test.yml | 75 ++++++ dotnet/README.md | 1 + .../AnthropicChatCompletionFixture.cs | 20 +- .../AnthropicSkillsIntegrationTests.cs | 40 ++- .../ConsoleAppSamplesValidation.cs | 42 ++- .../ExternalClientTests.cs | 10 +- .../SamplesValidationBase.cs | 2 +- .../WorkflowConsoleAppSamplesValidation.cs | 16 +- .../SamplesValidation.cs | 26 +- .../integration_test_report/aggregate.py | 254 +++++++++++++++--- 10 files changed, 370 insertions(+), 116 deletions(-) diff --git a/.github/workflows/dotnet-build-and-test.yml b/.github/workflows/dotnet-build-and-test.yml index 2d05ac9a02..0b589e2c55 100644 --- a/.github/workflows/dotnet-build-and-test.yml +++ b/.github/workflows/dotnet-build-and-test.yml @@ -273,6 +273,8 @@ jobs: -c ${{ matrix.configuration }} ` --no-build -v Normal ` --report-xunit-trx ` + --report-junit ` + --results-directory ../IntegrationTestResults/ ` --ignore-exit-code 8 ` --filter-not-trait "Category=IntegrationDisabled" ` --filter-not-trait "Category=FoundryHostedAgents" ` @@ -294,6 +296,10 @@ jobs: AZURE_AI_PROJECT_ENDPOINT: ${{ vars.AZURE_AI_PROJECT_ENDPOINT }} AZURE_AI_MODEL_DEPLOYMENT_NAME: ${{ vars.AZURE_AI_MODEL_DEPLOYMENT_NAME }} AZURE_AI_BING_CONNECTION_ID: ${{ vars.AZURE_AI_BING_CONNECTION_ID }} + # Anthropic Models + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + ANTHROPIC_CHAT_MODEL_NAME: ${{ vars.ANTHROPIC_CHAT_MODEL_NAME }} + ANTHROPIC_REASONING_MODEL_NAME: ${{ vars.ANTHROPIC_REASONING_MODEL_NAME }} # Generate test reports and check coverage - name: Generate test reports @@ -316,6 +322,14 @@ jobs: shell: pwsh run: ./dotnet/eng/scripts/dotnet-check-coverage.ps1 -JsonReportPath "TestResults/Reports/Summary.json" -CoverageThreshold $env:COVERAGE_THRESHOLD + - name: Upload integration test results + if: always() && github.event_name != 'pull_request' && matrix.integration-tests + uses: actions/upload-artifact@v7 + with: + name: dotnet-test-results-${{ matrix.targetFramework }}-${{ matrix.os }} + path: IntegrationTestResults/**/*.junit + if-no-files-found: ignore + # The Foundry hosted-agent IT is costly (it builds a container, pushes to ACR, and provisions # live agents on a separate Foundry project). Running it in its own job keeps the overall # workflow time roughly flat: it executes in parallel to dotnet-build and dotnet-test and is @@ -456,3 +470,64 @@ jobs: uses: actions/github-script@v8 with: script: core.setFailed('Integration Tests Cancelled!') + + # Integration test trend report (aggregates JUnit XML results from dotnet test jobs) + dotnet-integration-test-report: + name: Integration Test Report + if: > + always() && + github.event_name != 'pull_request' && + (contains(join(needs.*.result, ','), 'success') || + contains(join(needs.*.result, ','), 'failure')) + needs: [dotnet-test] + runs-on: ubuntu-latest + defaults: + run: + working-directory: python + steps: + - uses: actions/checkout@v6 + with: + persist-credentials: false + sparse-checkout: | + .github/actions/python-setup + python + - name: Set up python and install the project + uses: ./.github/actions/python-setup + with: + python-version: "3.13" + os: ${{ runner.os }} + - name: Download all test results from current run + uses: actions/download-artifact@v4 + with: + pattern: dotnet-test-results-* + path: dotnet-test-results/ + - name: Restore report history cache + uses: actions/cache/restore@v4 + with: + path: python/dotnet-integration-report-history.json + key: dotnet-integration-report-history-${{ github.run_id }} + restore-keys: | + dotnet-integration-report-history- + - name: Generate trend report + run: > + uv run python scripts/integration_test_report/aggregate.py + ../dotnet-test-results/ + dotnet-integration-report-history.json + dotnet-integration-test-report.md + - name: Post to Job Summary + if: always() + run: cat dotnet-integration-test-report.md >> $GITHUB_STEP_SUMMARY + - name: Save report history cache + if: always() + uses: actions/cache/save@v4 + with: + path: python/dotnet-integration-report-history.json + key: dotnet-integration-report-history-${{ github.run_id }} + - name: Upload trend report + if: always() + uses: actions/upload-artifact@v7 + with: + name: dotnet-integration-test-report + path: | + python/dotnet-integration-test-report.md + python/dotnet-integration-report-history.json diff --git a/dotnet/README.md b/dotnet/README.md index 328dfdf684..2edb402a94 100644 --- a/dotnet/README.md +++ b/dotnet/README.md @@ -33,3 +33,4 @@ Console.WriteLine(await agent.RunAsync("Write a haiku about Microsoft Agent Fram - [Design Documents](../docs/design) - [Architectural Decision Records](../docs/decisions) - [MSFT Learn Docs](https://learn.microsoft.com/agent-framework/overview/agent-framework-overview) + diff --git a/dotnet/tests/AnthropicChatCompletion.IntegrationTests/AnthropicChatCompletionFixture.cs b/dotnet/tests/AnthropicChatCompletion.IntegrationTests/AnthropicChatCompletionFixture.cs index af98629237..7c2a0c3b6c 100644 --- a/dotnet/tests/AnthropicChatCompletion.IntegrationTests/AnthropicChatCompletionFixture.cs +++ b/dotnet/tests/AnthropicChatCompletion.IntegrationTests/AnthropicChatCompletionFixture.cs @@ -17,9 +17,6 @@ namespace AnthropicChatCompletion.IntegrationTests; public class AnthropicChatCompletionFixture : IChatClientAgentFixture { - // All tests for Anthropic are intended to be ran locally as the CI pipeline for Anthropic is not setup. - internal const string SkipReason = "Integrations tests for local execution only"; - private readonly bool _useReasoningModel; private readonly bool _useBeta; @@ -105,7 +102,22 @@ public class AnthropicChatCompletionFixture : IChatClientAgentFixture public async ValueTask InitializeAsync() { - Assert.SkipWhen(SkipReason is not null, SkipReason ?? string.Empty); + // Temporarily disabled: Anthropic SDK has a binary incompatibility with the current + // Microsoft.Extensions.AI version (WebSearchToolResultContent.Results method not found). + // See: https://github.com/microsoft/agent-framework/pull/5515 + Assert.Skip("Anthropic integration tests temporarily disabled due to SDK incompatibility with Microsoft.Extensions.AI"); + + try + { + _ = TestConfiguration.GetRequiredValue(TestSettings.AnthropicApiKey); + _ = TestConfiguration.GetRequiredValue(TestSettings.AnthropicChatModelName); + _ = TestConfiguration.GetRequiredValue(TestSettings.AnthropicReasoningModelName); + } + catch (InvalidOperationException ex) + { + Assert.Skip("Anthropic configuration could not be loaded. Error:" + ex.Message); + } + this._agent = await this.CreateChatClientAgentAsync(); } diff --git a/dotnet/tests/AnthropicChatCompletion.IntegrationTests/AnthropicSkillsIntegrationTests.cs b/dotnet/tests/AnthropicChatCompletion.IntegrationTests/AnthropicSkillsIntegrationTests.cs index 452b0c6cf2..82b3511993 100644 --- a/dotnet/tests/AnthropicChatCompletion.IntegrationTests/AnthropicSkillsIntegrationTests.cs +++ b/dotnet/tests/AnthropicChatCompletion.IntegrationTests/AnthropicSkillsIntegrationTests.cs @@ -1,5 +1,6 @@ // Copyright (c) Microsoft. All rights reserved. +using System; using System.Threading.Tasks; using AgentConformance.IntegrationTests.Support; using Anthropic; @@ -17,19 +18,28 @@ namespace AnthropicChatCompletion.IntegrationTests; /// Integration tests for Anthropic Skills functionality. /// These tests are designed to be run locally with a valid Anthropic API key. /// +/// +/// Temporarily disabled due to Anthropic SDK binary incompatibility with +/// the current Microsoft.Extensions.AI version (WebSearchToolResultContent.Results). +/// +[Trait("Category", "IntegrationDisabled")] public sealed class AnthropicSkillsIntegrationTests { - // All tests for Anthropic are intended to be ran locally as the CI pipeline for Anthropic is not setup. - private const string SkipReason = "Integrations tests for local execution only"; - [Fact] public async Task CreateAgentWithPptxSkillAsync() { - Assert.SkipWhen(SkipReason is not null, SkipReason ?? string.Empty); - - // Arrange - AnthropicClient anthropicClient = new() { ApiKey = TestConfiguration.GetRequiredValue(TestSettings.AnthropicApiKey) }; - string model = TestConfiguration.GetRequiredValue(TestSettings.AnthropicChatModelName); + AnthropicClient? anthropicClient; + string? model; + try + { + anthropicClient = new() { ApiKey = TestConfiguration.GetRequiredValue(TestSettings.AnthropicApiKey) }; + model = TestConfiguration.GetRequiredValue(TestSettings.AnthropicChatModelName); + } + catch (InvalidOperationException ex) + { + Assert.Skip("Anthropic configuration could not be loaded. Error:" + ex.Message); + return; + } BetaSkillParams pptxSkill = new() { @@ -56,10 +66,16 @@ public sealed class AnthropicSkillsIntegrationTests [Fact] public async Task ListAnthropicManagedSkillsAsync() { - Assert.SkipWhen(SkipReason is not null, SkipReason ?? string.Empty); - - // Arrange - AnthropicClient anthropicClient = new() { ApiKey = TestConfiguration.GetRequiredValue(TestSettings.AnthropicApiKey) }; + AnthropicClient? anthropicClient; + try + { + anthropicClient = new() { ApiKey = TestConfiguration.GetRequiredValue(TestSettings.AnthropicApiKey) }; + } + catch (InvalidOperationException ex) + { + Assert.Skip("Anthropic configuration could not be loaded. Error:" + ex.Message); + return; + } // Act SkillListPage skills = await anthropicClient.Beta.Skills.List( diff --git a/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/ConsoleAppSamplesValidation.cs b/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/ConsoleAppSamplesValidation.cs index 7b8fa3a8f9..5e1142f027 100644 --- a/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/ConsoleAppSamplesValidation.cs +++ b/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/ConsoleAppSamplesValidation.cs @@ -13,8 +13,6 @@ namespace Microsoft.Agents.AI.DurableTask.IntegrationTests; [Trait("Category", "SampleValidation")] public sealed class ConsoleAppSamplesValidation(ITestOutputHelper outputHelper) : SamplesValidationBase(outputHelper) { - private const string SkipFlakyTimingTest = "Flaky: timing-dependent LLM test, see https://github.com/microsoft/agent-framework/issues/4971"; - private static readonly string s_samplesPath = Path.GetFullPath( Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "..", "..", "..", "..", "..", "samples", "04-hosting", "DurableAgents", "ConsoleApps")); @@ -69,7 +67,7 @@ public sealed class ConsoleAppSamplesValidation(ITestOutputHelper outputHelper) }); } - [Fact] + [RetryFact(2, 5000)] public async Task SingleAgentOrchestrationChainingSampleValidationAsync() { using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(); @@ -105,7 +103,7 @@ public sealed class ConsoleAppSamplesValidation(ITestOutputHelper outputHelper) }); } - [Fact] + [RetryFact(2, 5000)] public async Task MultiAgentConcurrencySampleValidationAsync() { using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(); @@ -160,7 +158,7 @@ public sealed class ConsoleAppSamplesValidation(ITestOutputHelper outputHelper) }); } - [Fact] + [RetryFact(2, 5000)] public async Task MultiAgentConditionalSampleValidationAsync() { using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(); @@ -237,14 +235,14 @@ public sealed class ConsoleAppSamplesValidation(ITestOutputHelper outputHelper) Assert.True(foundSuccess, "Orchestration did not complete successfully."); } - [Fact(Skip = SkipFlakyTimingTest)] + [RetryFact(2, 5000)] public async Task SingleAgentOrchestrationHITLSampleValidationAsync() { string samplePath = Path.Combine(s_samplesPath, "05_AgentOrchestration_HITL"); await this.RunSampleTestAsync(samplePath, async (process, logs) => { - using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(); + using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(TimeSpan.FromSeconds(180)); // Start the HITL orchestration following the happy path from README await this.WriteInputAsync(process, "The Future of Artificial Intelligence", testTimeoutCts.Token); @@ -260,7 +258,7 @@ public sealed class ConsoleAppSamplesValidation(ITestOutputHelper outputHelper) while ((line = this.ReadLogLine(logs, testTimeoutCts.Token)) != null) { // Look for notification that content is ready. The first time we see this, we should send a rejection. - // The second time we see this, we should send approval. + // Subsequent times we see this, we should send approval (LLM may produce extra review cycles). if (line.Contains("Content is ready for review", StringComparison.OrdinalIgnoreCase)) { if (!rejectionSent) @@ -275,20 +273,15 @@ public sealed class ConsoleAppSamplesValidation(ITestOutputHelper outputHelper) testTimeoutCts.Token); rejectionSent = true; } - else if (!approvalSent) + else { - // Prompt: Approve? (y/n): + // Approve any subsequent draft (LLM non-determinism may produce extra review cycles) await this.WriteInputAsync(process, "y", testTimeoutCts.Token); // Prompt: Feedback (optional): await this.WriteInputAsync(process, "Looks good!", testTimeoutCts.Token); approvalSent = true; } - else - { - // This should never happen - Assert.Fail("Unexpected message found."); - } } // Look for success message @@ -311,14 +304,14 @@ public sealed class ConsoleAppSamplesValidation(ITestOutputHelper outputHelper) }); } - [Fact(Skip = SkipFlakyTimingTest)] + [RetryFact(2, 5000)] public async Task LongRunningToolsSampleValidationAsync() { string samplePath = Path.Combine(s_samplesPath, "06_LongRunningTools"); await this.RunSampleTestAsync(samplePath, async (process, logs) => { // This test takes a bit longer to run due to the multiple agent interactions and the lengthy content generation. - using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(TimeSpan.FromSeconds(90)); + using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(TimeSpan.FromSeconds(180)); // Test starting an agent that schedules a content generation orchestration await this.WriteInputAsync( @@ -335,7 +328,7 @@ public sealed class ConsoleAppSamplesValidation(ITestOutputHelper outputHelper) while ((line = this.ReadLogLine(logs, testTimeoutCts.Token)) != null) { // Look for notification that content is ready. The first time we see this, we should send a rejection. - // The second time we see this, we should send approval. + // Subsequent times we see this, we should send approval (LLM may produce extra review cycles). if (line.Contains("NOTIFICATION: Please review the following content for approval", StringComparison.OrdinalIgnoreCase)) { // Wait for the notification to be fully written to the console @@ -350,20 +343,15 @@ public sealed class ConsoleAppSamplesValidation(ITestOutputHelper outputHelper) testTimeoutCts.Token); rejectionSent = true; } - else if (!approvalSent) + else { - // Approve the content. Note that we need to send a newline character to the console first before sending the input. + // Approve any subsequent draft (LLM non-determinism may produce extra review cycles) await this.WriteInputAsync( process, "\nApprove the content", testTimeoutCts.Token); approvalSent = true; } - else - { - // This should never happen - Assert.Fail("Unexpected message found."); - } } // Look for success message @@ -396,14 +384,14 @@ public sealed class ConsoleAppSamplesValidation(ITestOutputHelper outputHelper) }); } - [Fact(Skip = SkipFlakyTimingTest)] + [RetryFact(2, 5000)] public async Task ReliableStreamingSampleValidationAsync() { string samplePath = Path.Combine(s_samplesPath, "07_ReliableStreaming"); await this.RunSampleTestAsync(samplePath, async (process, logs) => { // This test takes a bit longer to run due to the multiple agent interactions and the lengthy content generation. - using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(TimeSpan.FromSeconds(90)); + using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(TimeSpan.FromSeconds(150)); // Test the agent endpoint with a simple prompt await this.WriteInputAsync(process, "Plan a 5-day trip to Seattle. Include daily activities.", testTimeoutCts.Token); diff --git a/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/ExternalClientTests.cs b/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/ExternalClientTests.cs index 134a12e688..aa1edab7da 100644 --- a/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/ExternalClientTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/ExternalClientTests.cs @@ -19,11 +19,9 @@ namespace Microsoft.Agents.AI.DurableTask.IntegrationTests; [Trait("Category", "Integration")] public sealed class ExternalClientTests(ITestOutputHelper outputHelper) : IDisposable { - private const string SkipFlakyTimingTest = "Flaky: timing-dependent LLM test, see https://github.com/microsoft/agent-framework/issues/4971"; - private static readonly TimeSpan s_defaultTimeout = Debugger.IsAttached ? TimeSpan.FromMinutes(5) - : TimeSpan.FromSeconds(60); + : TimeSpan.FromSeconds(120); private static readonly IConfiguration s_configuration = new ConfigurationBuilder() @@ -38,7 +36,7 @@ public sealed class ExternalClientTests(ITestOutputHelper outputHelper) : IDispo public void Dispose() => this._cts.Dispose(); - [Fact] + [RetryFact(2, 5000)] public async Task SimplePromptAsync() { // Setup @@ -77,7 +75,7 @@ public sealed class ExternalClientTests(ITestOutputHelper outputHelper) : IDispo Assert.Contains(agentLogs, log => log.EventId.Name == "LogAgentResponse"); } - [Fact(Skip = SkipFlakyTimingTest)] + [RetryFact(2, 5000)] public async Task CallFunctionToolsAsync() { int weatherToolInvocationCount = 0; @@ -129,7 +127,7 @@ public sealed class ExternalClientTests(ITestOutputHelper outputHelper) : IDispo Assert.Equal(1, packingListToolInvocationCount); } - [Fact(Skip = SkipFlakyTimingTest)] + [RetryFact(2, 5000)] public async Task CallLongRunningFunctionToolsAsync() { [Description("Starts a greeting workflow and returns the workflow instance ID")] diff --git a/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/SamplesValidationBase.cs b/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/SamplesValidationBase.cs index f5ecf0354d..3f01b83e54 100644 --- a/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/SamplesValidationBase.cs +++ b/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/SamplesValidationBase.cs @@ -217,7 +217,7 @@ public abstract class SamplesValidationBase : IAsyncLifetime /// protected CancellationTokenSource CreateTestTimeoutCts(TimeSpan? timeout = null) { - TimeSpan testTimeout = Debugger.IsAttached ? TimeSpan.FromMinutes(5) : timeout ?? TimeSpan.FromSeconds(60); + TimeSpan testTimeout = Debugger.IsAttached ? TimeSpan.FromMinutes(5) : timeout ?? TimeSpan.FromSeconds(120); return new CancellationTokenSource(testTimeout); } diff --git a/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/WorkflowConsoleAppSamplesValidation.cs b/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/WorkflowConsoleAppSamplesValidation.cs index f137e4abd9..390b3586ce 100644 --- a/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/WorkflowConsoleAppSamplesValidation.cs +++ b/dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/WorkflowConsoleAppSamplesValidation.cs @@ -22,7 +22,7 @@ public sealed class WorkflowConsoleAppSamplesValidation(ITestOutputHelper output /// protected override string TaskHubPrefix => "workflow"; - [Fact] + [RetryFact(2, 5000)] public async Task SequentialWorkflowSampleValidationAsync() { using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(s_testTimeout); @@ -71,7 +71,7 @@ public sealed class WorkflowConsoleAppSamplesValidation(ITestOutputHelper output }); } - [Fact] + [RetryFact(2, 5000)] public async Task ConcurrentWorkflowSampleValidationAsync() { using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(s_testTimeout); @@ -120,7 +120,7 @@ public sealed class WorkflowConsoleAppSamplesValidation(ITestOutputHelper output }); } - [Fact] + [RetryFact(2, 5000)] public async Task ConditionalEdgesWorkflowSampleValidationAsync() { using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(s_testTimeout); @@ -182,7 +182,7 @@ public sealed class WorkflowConsoleAppSamplesValidation(ITestOutputHelper output } } - [Fact] + [RetryFact(2, 5000)] public async Task WorkflowEventsSampleValidationAsync() { using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(s_testTimeout); @@ -278,7 +278,7 @@ public sealed class WorkflowConsoleAppSamplesValidation(ITestOutputHelper output }); } - [Fact] + [RetryFact(2, 5000)] public async Task WorkflowSharedStateSampleValidationAsync() { using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(s_testTimeout); @@ -376,7 +376,7 @@ public sealed class WorkflowConsoleAppSamplesValidation(ITestOutputHelper output }); } - [Fact] + [RetryFact(2, 5000)] public async Task SubWorkflowsSampleValidationAsync() { using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(s_testTimeout); @@ -452,7 +452,7 @@ public sealed class WorkflowConsoleAppSamplesValidation(ITestOutputHelper output }); } - [Fact] + [RetryFact(2, 5000)] public async Task WorkflowHITLSampleValidationAsync() { using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(s_testTimeout); @@ -505,7 +505,7 @@ public sealed class WorkflowConsoleAppSamplesValidation(ITestOutputHelper output }); } - [Fact] + [RetryFact(2, 5000)] public async Task WorkflowAndAgentsSampleValidationAsync() { using CancellationTokenSource testTimeoutCts = this.CreateTestTimeoutCts(s_testTimeout); diff --git a/dotnet/tests/Microsoft.Agents.AI.Hosting.AzureFunctions.IntegrationTests/SamplesValidation.cs b/dotnet/tests/Microsoft.Agents.AI.Hosting.AzureFunctions.IntegrationTests/SamplesValidation.cs index 078b6af790..be9d2b7434 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Hosting.AzureFunctions.IntegrationTests/SamplesValidation.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Hosting.AzureFunctions.IntegrationTests/SamplesValidation.cs @@ -15,8 +15,6 @@ namespace Microsoft.Agents.AI.Hosting.AzureFunctions.IntegrationTests; [Trait("Category", "SampleValidation")] public sealed class SamplesValidation(ITestOutputHelper outputHelper) : IAsyncLifetime { - private const string SkipFlakyTimingTest = "Flaky: timing-dependent LLM test, see https://github.com/microsoft/agent-framework/issues/4971"; - private const string AzureFunctionsPort = "7071"; private const string AzuritePort = "10000"; private const string DtsPort = "8080"; @@ -37,7 +35,7 @@ public sealed class SamplesValidation(ITestOutputHelper outputHelper) : IAsyncLi .Build(); private static bool s_infrastructureStarted; - private static readonly TimeSpan s_orchestrationTimeout = TimeSpan.FromMinutes(2); + private static readonly TimeSpan s_orchestrationTimeout = TimeSpan.FromMinutes(3); // In CI, `dotnet run` builds the Functions project from scratch before the host starts, so 60s is not enough. private static readonly TimeSpan s_functionsReadyTimeout = TimeSpan.FromSeconds(180); @@ -62,7 +60,7 @@ public sealed class SamplesValidation(ITestOutputHelper outputHelper) : IAsyncLi await Task.CompletedTask; } - [Fact] + [RetryFact(2, 5000)] public async Task SingleAgentSampleValidationAsync() { string samplePath = Path.Combine(s_samplesPath, "01_SingleAgent"); @@ -107,7 +105,7 @@ public sealed class SamplesValidation(ITestOutputHelper outputHelper) : IAsyncLi }); } - [Fact] + [Fact(Skip = "Flaky: LLM non-determinism can produce null orchestration results")] public async Task SingleAgentOrchestrationChainingSampleValidationAsync() { string samplePath = Path.Combine(s_samplesPath, "02_AgentOrchestration_Chaining"); @@ -150,7 +148,7 @@ public sealed class SamplesValidation(ITestOutputHelper outputHelper) : IAsyncLi }); } - [Fact] + [RetryFact(2, 5000)] public async Task MultiAgentOrchestrationConcurrentSampleValidationAsync() { string samplePath = Path.Combine(s_samplesPath, "03_AgentOrchestration_Concurrency"); @@ -200,7 +198,7 @@ public sealed class SamplesValidation(ITestOutputHelper outputHelper) : IAsyncLi }); } - [Fact] + [RetryFact(2, 5000)] public async Task MultiAgentOrchestrationConditionalsSampleValidationAsync() { string samplePath = Path.Combine(s_samplesPath, "04_AgentOrchestration_Conditionals"); @@ -218,7 +216,7 @@ public sealed class SamplesValidation(ITestOutputHelper outputHelper) : IAsyncLi }); } - [Fact] + [RetryFact(2, 5000)] public async Task SingleAgentOrchestrationHITLSampleValidationAsync() { string samplePath = Path.Combine(s_samplesPath, "05_AgentOrchestration_HITL"); @@ -274,7 +272,7 @@ public sealed class SamplesValidation(ITestOutputHelper outputHelper) : IAsyncLi }); } - [Fact(Skip = SkipFlakyTimingTest)] + [RetryFact(2, 5000)] public async Task LongRunningToolsSampleValidationAsync() { string samplePath = Path.Combine(s_samplesPath, "06_LongRunningTools"); @@ -316,7 +314,7 @@ public sealed class SamplesValidation(ITestOutputHelper outputHelper) : IAsyncLi } }, message: "Orchestration is requesting human feedback", - timeout: TimeSpan.FromSeconds(60)); + timeout: TimeSpan.FromSeconds(180)); // Approve the content Uri approvalUri = new($"{runAgentUri}?thread_id={sessionId}"); @@ -336,7 +334,7 @@ public sealed class SamplesValidation(ITestOutputHelper outputHelper) : IAsyncLi } }, message: "Content published notification is logged", - timeout: TimeSpan.FromSeconds(60)); + timeout: TimeSpan.FromSeconds(180)); // Verify the final orchestration status by asking the agent for the status Uri statusUri = new($"{runAgentUri}?thread_id={sessionId}"); @@ -360,11 +358,11 @@ public sealed class SamplesValidation(ITestOutputHelper outputHelper) : IAsyncLi return isCompleted && hasContent; }, message: "Orchestration is completed", - timeout: TimeSpan.FromSeconds(60)); + timeout: TimeSpan.FromSeconds(180)); }); } - [Fact] + [RetryFact(2, 5000)] public async Task AgentAsMcpToolAsync() { string samplePath = Path.Combine(s_samplesPath, "07_AgentAsMcpTool"); @@ -404,7 +402,7 @@ public sealed class SamplesValidation(ITestOutputHelper outputHelper) : IAsyncLi }); } - [Fact(Skip = SkipFlakyTimingTest)] + [RetryFact(2, 5000)] public async Task ReliableStreamingSampleValidationAsync() { string samplePath = Path.Combine(s_samplesPath, "08_ReliableStreaming"); diff --git a/python/scripts/integration_test_report/aggregate.py b/python/scripts/integration_test_report/aggregate.py index e803add730..708f47fcf0 100644 --- a/python/scripts/integration_test_report/aggregate.py +++ b/python/scripts/integration_test_report/aggregate.py @@ -2,16 +2,18 @@ """Aggregate per-provider JUnit XML test results and generate a trend report. -Parses ``pytest.xml`` (JUnit XML) files produced by each CI job, merges them -into a single run, combines with historical data, and generates a markdown -trend table — the same pattern used by ``scripts/sample_validation/aggregate.py``. +Parses JUnit XML files produced by CI jobs — both ``pytest.xml`` (Python) and +xunit v3 ``*.junit`` (dotnet) — merges them into a single run, combines +with historical data, and generates a markdown trend table. Usage (from CI): python aggregate.py -The reports directory is expected to contain subdirectories named -``test-results-/`` each containing a ``pytest.xml`` file -(created by ``actions/download-artifact``). +The reports directory is expected to contain artifact subdirectories. Two +layouts are supported: + +- **Python (pytest):** ``test-results-/pytest.xml`` +- **Dotnet (xunit):** ``dotnet-test-results--/*.junit`` """ from __future__ import annotations @@ -46,9 +48,21 @@ def _format_run_label(timestamp: str) -> str: def _derive_provider(directory_name: str) -> str: """Derive a provider label from a report directory name. - ``test-results-openai`` → ``OpenAI`` - ``test-results-azure-openai`` → ``Azure OpenAI`` + Handles both Python and dotnet naming conventions: + - ``test-results-openai`` → ``OpenAI`` + - ``test-results-azure-openai`` → ``Azure OpenAI`` + - ``dotnet-test-results-net10.0-ubuntu-latest`` → ``net10.0 (ubuntu)`` """ + # Dotnet convention: dotnet-test-results-- + if directory_name.startswith("dotnet-test-results-"): + raw = directory_name.replace("dotnet-test-results-", "") + # e.g. "net10.0-ubuntu-latest" → framework="net10.0", os="ubuntu-latest" + parts = raw.split("-", 1) + framework = parts[0] + os_label = parts[1].split("-")[0] if len(parts) > 1 else "" + return f"{framework} ({os_label})" if os_label else framework + + # Python convention: test-results- raw = directory_name.replace("test-results-", "") known = { "openai": "OpenAI", @@ -102,11 +116,21 @@ def _parse_junit_xml(xml_path: Path) -> list[dict[str, str]]: # it appends the class name, e.g.: # "packages.foundry.tests.foundry.test_foundry_embedding_client.TestFoundryEmbeddingIntegration" # We want the file-level module: "test_foundry_embedding_client" + # + # xunit (dotnet) writes classname as the full C# type, e.g.: + # "OpenAIChatCompletion.IntegrationTests.ChatCompletionTests" + # We want the project prefix: "OpenAIChatCompletion" if classname: parts = classname.rsplit(".", 2) # If the last segment starts with uppercase it's a class name — take the one before it if len(parts) >= 2 and parts[-1][0:1].isupper(): - module = parts[-2] + # For dotnet: if the penultimate part is "IntegrationTests" or "UnitTests", + # use the part before that (the project name) instead + if parts[-2] in ("IntegrationTests", "UnitTests") and len(parts) >= 3: + # parts[0] may contain dots — take the last segment of it + module = parts[0].rsplit(".", 1)[-1] + else: + module = parts[-2] else: module = parts[-1] else: @@ -148,28 +172,61 @@ def _parse_junit_xml(xml_path: Path) -> list[dict[str, str]]: # --------------------------------------------------------------------------- +def _discover_xml_files(reports_dir: Path) -> list[tuple[str, Path]]: + """Discover JUnit XML test result files in artifact subdirectories. + + Handles two directory layouts: + - **Python (pytest):** ``test-results-/pytest.xml`` + - **Dotnet (xunit):** ``dotnet-test-results--/*.junit`` + + Returns: + List of ``(directory_name, xml_path)`` tuples. + """ + xml_files: list[tuple[str, Path]] = [] + if not reports_dir.is_dir(): + return xml_files + + for subdir in sorted(reports_dir.iterdir()): + if not subdir.is_dir(): + continue + + # Python layout: single pytest.xml per artifact + pytest_xml = subdir / "pytest.xml" + if pytest_xml.exists(): + xml_files.append((subdir.name, pytest_xml)) + continue + + # Dotnet layout: multiple *.junit files per artifact + junit_files = sorted(subdir.rglob("*.junit")) + for jf in junit_files: + xml_files.append((subdir.name, jf)) + + # Fallback: any .xml file that looks like JUnit (not .trx, not cobertura) + if not junit_files: + for xf in sorted(subdir.rglob("*.xml")): + if xf.suffix == ".xml" and not xf.name.endswith(".cobertura.xml"): + xml_files.append((subdir.name, xf)) + + return xml_files + + def load_current_run(reports_dir: Path) -> dict[str, Any]: """Load per-provider JUnit XML reports from the current CI run and merge. + Supports both pytest (Python) and xunit v3 (dotnet) JUnit XML formats. + Args: - reports_dir: Directory containing ``test-results-/`` subdirs. + reports_dir: Directory containing artifact subdirectories with XML reports. Returns: Merged run dict with ``timestamp``, ``summary``, ``results``. """ combined_results: dict[str, dict[str, str]] = {} # nodeid → {status, provider} - # actions/download-artifact creates: reports_dir/test-results-openai/pytest.xml - xml_files: list[tuple[str, Path]] = [] - if reports_dir.is_dir(): - for subdir in sorted(reports_dir.iterdir()): - if subdir.is_dir(): - xml_file = subdir / "pytest.xml" - if xml_file.exists(): - xml_files.append((subdir.name, xml_file)) + xml_files = _discover_xml_files(reports_dir) if not xml_files: - print(f"Warning: No pytest.xml files found in {reports_dir}") + print(f"Warning: No JUnit XML files found in {reports_dir}") return { "timestamp": datetime.now(timezone.utc).isoformat(), "summary": { @@ -181,19 +238,42 @@ def load_current_run(reports_dir: Path) -> dict[str, Any]: "results": {}, } + # Dotnet tests always run under multiple frameworks, so we always + # qualify their keys with the provider to ensure deterministic, + # stable keys across runs regardless of file parse order. + is_dotnet = any(d.startswith("dotnet-test-results-") for d, _ in xml_files) + for dir_name, xml_file in xml_files: print(f" Loading: {xml_file}") provider = _derive_provider(dir_name) tests = _parse_junit_xml(xml_file) for test in tests: - combined_results[test["nodeid"]] = { + raw_id = test["nodeid"] + key = f"{provider}::{raw_id}" if is_dotnet else raw_id + + combined_results[key] = { "status": test["status"], "provider": provider, "module": test.get("module", ""), } - # Build summary counts using mutually exclusive status buckets. - # Errors are folded into the failed count for display purposes. + # Build per-provider summary counts so the report can show one row per + # framework (dotnet) or per provider (Python). + provider_counts: dict[str, dict[str, int]] = {} + for r in combined_results.values(): + prov = r.get("provider", "Unknown") + if prov not in provider_counts: + provider_counts[prov] = {"total": 0, "passed": 0, "failed": 0, "skipped": 0} + provider_counts[prov]["total"] += 1 + st = r["status"] + if st == "passed": + provider_counts[prov]["passed"] += 1 + elif st in ("failed", "error"): + provider_counts[prov]["failed"] += 1 + elif st == "skipped": + provider_counts[prov]["skipped"] += 1 + + # Overall summary (sum across all providers). statuses = [r["status"] for r in combined_results.values()] summary = { "total": len(statuses), @@ -205,6 +285,7 @@ def load_current_run(reports_dir: Path) -> dict[str, Any]: return { "timestamp": datetime.now(timezone.utc).isoformat(), "summary": summary, + "provider_summaries": provider_counts, "results": combined_results, } @@ -253,7 +334,29 @@ def generate_trend_report(runs: list[dict[str, Any]]) -> str: "", ] - # --- Overall status table (most recent first) --- + # Detect whether this is a dotnet report (provider-qualified keys). + is_dotnet = False + for run in runs: + provider_sums = run.get("provider_summaries", {}) + if any(p.startswith("net") for p in provider_sums): + is_dotnet = True + break + + if is_dotnet: + _generate_dotnet_report(lines, runs) + else: + _generate_python_report(lines, runs) + + lines.append("") + lines.append("**Legend:** ✅ Passed · ❌ Failed · ⏭️ Skipped · ⚠️ Expected Failure (xfail) · N/A Not available") + lines.append("") + + return "\n".join(lines) + + +def _generate_python_report(lines: list[str], runs: list[dict[str, Any]]) -> None: + """Generate the original single-table Python report format.""" + # --- Overall status table --- lines.append("## Overall Status (Last 5 Runs)") lines.append("") lines.append("| Run | Total | ✅ Passed | ❌ Failed | ⏭️ Skipped |") @@ -276,27 +379,91 @@ def generate_trend_report(runs: list[dict[str, Any]]) -> str: lines.append("") - # --- Per-test results table --- - lines.append("## Per-Test Results") - lines.append("") + # --- Single per-test results table --- + _generate_per_test_table(lines, runs, "## Per-Test Results") - # Collect all test nodeids, providers, and modules across all runs - all_tests: dict[str, str] = {} # nodeid → provider (from most recent run) - all_modules: dict[str, str] = {} # nodeid → module (from most recent run) + +def _generate_dotnet_report(lines: list[str], runs: list[dict[str, Any]]) -> None: + """Generate per-framework tables for dotnet (net10.0, net472, etc.).""" + # Collect all providers seen across all runs, sorted for stable ordering + all_providers: set[str] = set() + for run in runs: + all_providers.update(run.get("provider_summaries", {}).keys()) + providers = sorted(all_providers) + + for provider in providers: + lines.append(f"## {provider}") + lines.append("") + + # --- Per-provider summary table --- + lines.append("| Run | Total | ✅ Passed | ❌ Failed | ⏭️ Skipped |") + lines.append("|-----|-------|-----------|-----------|------------|") + + for run in reversed(runs): + ps = run.get("provider_summaries", {}).get(provider, {}) + total = ps.get("total", 0) + label = _format_run_label(run["timestamp"]) + if total == 0: + lines.append(f"| {label} | N/A | N/A | N/A | N/A |") + else: + lines.append( + f"| {label} " + f"| {total} " + f"| {ps.get('passed', 0)}/{total} " + f"| {ps.get('failed', 0)}/{total} " + f"| {ps.get('skipped', 0)}/{total} |" + ) + + for _ in range(MAX_HISTORY - len(runs)): + lines.append("| N/A | N/A | N/A | N/A | N/A |") + + lines.append("") + + # --- Per-test table filtered to this provider --- + _generate_per_test_table( + lines, runs, + heading=None, + provider_filter=provider, + ) + + +def _generate_per_test_table( + lines: list[str], + runs: list[dict[str, Any]], + heading: str | None = None, + provider_filter: str | None = None, +) -> None: + """Emit a per-test trend table, optionally filtered to a single provider.""" + if heading: + lines.append(heading) + lines.append("") + + # Collect all test nodeids (and metadata) across all runs + all_tests: dict[str, str] = {} # nodeid → provider + all_modules: dict[str, str] = {} # nodeid → module for run in runs: for nodeid, info in run.get("results", {}).items(): - provider = info.get("provider", "Unknown") if isinstance(info, dict) else "Unknown" - module = info.get("module", "") if isinstance(info, dict) else "" - all_tests[nodeid] = provider + if not isinstance(info, dict): + continue + prov = info.get("provider", "Unknown") + if provider_filter and prov != provider_filter: + continue + module = info.get("module", "") + all_tests[nodeid] = prov all_modules[nodeid] = module if not all_tests: lines.append("*No test results available.*") - return "\n".join(lines) + lines.append("") + return - # Build header (most recent run first) - header = "| Test | File | Provider |" - separator = "|------|------|----------|" + # Build header + if provider_filter: + header = "| Test | File |" + separator = "|------|------|" + else: + header = "| Test | File | Provider |" + separator = "|------|------|----------|" for run in reversed(runs): label = _format_run_label(run["timestamp"]) header += f" {label} |" @@ -308,12 +475,15 @@ def generate_trend_report(runs: list[dict[str, Any]]) -> str: lines.append(header) lines.append(separator) - # Sort by provider then test name - for nodeid in sorted(all_tests, key=lambda n: (all_tests[n], n)): - provider = all_tests[nodeid] + # Sort by module then test name + for nodeid in sorted(all_tests, key=lambda n: (all_modules.get(n, ""), n)): module = all_modules.get(nodeid, "") short = _short_name(nodeid) - row = f"| `{short}` | `{module}` | {provider} |" + if provider_filter: + row = f"| `{short}` | `{module}` |" + else: + provider = all_tests[nodeid] + row = f"| `{short}` | `{module}` | {provider} |" for run in reversed(runs): result = run.get("results", {}).get(nodeid) @@ -330,10 +500,6 @@ def generate_trend_report(runs: list[dict[str, Any]]) -> str: lines.append(row) lines.append("") - lines.append("**Legend:** ✅ Passed · ❌ Failed · ⏭️ Skipped · ⚠️ Expected Failure (xfail) · N/A Not available") - lines.append("") - - return "\n".join(lines) # --------------------------------------------------------------------------- From d3518ad19d456fd0f0fe45be8a0f4e0968d8567e Mon Sep 17 00:00:00 2001 From: tuanaiseo Date: Fri, 8 May 2026 03:42:49 +0700 Subject: [PATCH 5/6] fix(security): non-thread-safe sequence number generation may cau (#5320) `SequenceNumber.Increment()` uses `this._sequenceNumber++` without synchronization. In concurrent streaming scenarios, this can produce race conditions and inconsistent sequencing, which may break event ordering guarantees and potentially allow response-mixing or state confusion. Affected files: SequenceNumber.cs Signed-off-by: tuanaiseo <221258316+tuanaiseo@users.noreply.github.com> Co-authored-by: Jacob Alber --- .../Responses/Streaming/SequenceNumber.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotnet/src/Microsoft.Agents.AI.Hosting.OpenAI/Responses/Streaming/SequenceNumber.cs b/dotnet/src/Microsoft.Agents.AI.Hosting.OpenAI/Responses/Streaming/SequenceNumber.cs index d119275f71..e125c4269e 100644 --- a/dotnet/src/Microsoft.Agents.AI.Hosting.OpenAI/Responses/Streaming/SequenceNumber.cs +++ b/dotnet/src/Microsoft.Agents.AI.Hosting.OpenAI/Responses/Streaming/SequenceNumber.cs @@ -13,5 +13,5 @@ internal sealed class SequenceNumber /// Gets the next sequence number. /// /// The next sequence number. - public int Increment() => this._sequenceNumber++; + public int Increment() => System.Threading.Interlocked.Increment(ref this._sequenceNumber) - 1; } From 3c1e2c40b89372e9ffb44c01d2f3f66412eda444 Mon Sep 17 00:00:00 2001 From: Hao-Xiong <143869985+XiongHaoTrigger@users.noreply.github.com> Date: Fri, 8 May 2026 05:05:41 +0800 Subject: [PATCH 6/6] Fix typo:sesionEleme -> sessionElement (#5674) Co-authored-by: Jacob Alber --- dotnet/samples/01-get-started/04_memory/Program.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dotnet/samples/01-get-started/04_memory/Program.cs b/dotnet/samples/01-get-started/04_memory/Program.cs index ebafc60b45..961066682a 100644 --- a/dotnet/samples/01-get-started/04_memory/Program.cs +++ b/dotnet/samples/01-get-started/04_memory/Program.cs @@ -50,12 +50,12 @@ Console.WriteLine(await agent.RunAsync("My name is Ruaidhrí", session)); Console.WriteLine(await agent.RunAsync("I am 20 years old", session)); // We can serialize the session. The serialized state will include the state of the memory component. -JsonElement sesionElement = await agent.SerializeSessionAsync(session); +JsonElement sessionElement = await agent.SerializeSessionAsync(session); Console.WriteLine("\n>> Use deserialized session with previously created memories\n"); // Later we can deserialize the session and continue the conversation with the previous memory component state. -var deserializedSession = await agent.DeserializeSessionAsync(sesionElement); +var deserializedSession = await agent.DeserializeSessionAsync(sessionElement); Console.WriteLine(await agent.RunAsync("What is my name and age?", deserializedSession)); Console.WriteLine("\n>> Read memories using memory component\n");