mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Merge branch 'main' into copilot/port-magentic-orchestration-sample
This commit is contained in:
@@ -299,6 +299,7 @@
|
||||
</Folder>
|
||||
<Folder Name="/Samples/03-workflows/Evaluation/">
|
||||
<Project Path="samples/03-workflows/Evaluation/Evaluation_WorkflowEval/Evaluation_WorkflowEval.csproj" />
|
||||
<Project Path="samples/03-workflows/Evaluation/Evaluation_WorkflowExpectedOutputs/Evaluation_WorkflowExpectedOutputs.csproj" />
|
||||
</Folder>
|
||||
<Folder Name="/Samples/04-hosting/">
|
||||
</Folder>
|
||||
|
||||
@@ -1,14 +1,14 @@
|
||||
<Project>
|
||||
<PropertyGroup>
|
||||
<!-- Central version prefix - applies to all nuget packages. -->
|
||||
<VersionPrefix>1.5.0</VersionPrefix>
|
||||
<VersionPrefix>1.6.0</VersionPrefix>
|
||||
<RCNumber>1</RCNumber>
|
||||
<DateSuffix>260507</DateSuffix>
|
||||
<DateSuffix>260512</DateSuffix>
|
||||
<PackageVersion Condition="'$(IsReleaseCandidate)' == 'true'">$(VersionPrefix)-rc$(RCNumber)</PackageVersion>
|
||||
<PackageVersion Condition="'$(IsReleaseCandidate)' != 'true' AND '$(VersionSuffix)' != ''">$(VersionPrefix)-$(VersionSuffix).$(DateSuffix).1</PackageVersion>
|
||||
<PackageVersion Condition="'$(IsReleaseCandidate)' != 'true' AND '$(VersionSuffix)' == ''">$(VersionPrefix)-preview.$(DateSuffix).1</PackageVersion>
|
||||
<PackageVersion Condition="'$(IsReleased)' == 'true'">$(VersionPrefix)</PackageVersion>
|
||||
<GitTag>1.5.0</GitTag>
|
||||
<GitTag>1.6.0</GitTag>
|
||||
|
||||
<Configurations>Debug;Release;Publish</Configurations>
|
||||
<IsPackable>true</IsPackable>
|
||||
|
||||
+16
@@ -0,0 +1,16 @@
|
||||
<Project Sdk="Microsoft.NET.Sdk">
|
||||
|
||||
<PropertyGroup>
|
||||
<OutputType>Exe</OutputType>
|
||||
<TargetFrameworks>net10.0</TargetFrameworks>
|
||||
|
||||
<Nullable>enable</Nullable>
|
||||
<ImplicitUsings>enable</ImplicitUsings>
|
||||
</PropertyGroup>
|
||||
|
||||
<ItemGroup>
|
||||
<ProjectReference Include="..\..\..\..\src\Microsoft.Agents.AI.Foundry\Microsoft.Agents.AI.Foundry.csproj" />
|
||||
<ProjectReference Include="..\..\..\..\src\Microsoft.Agents.AI.Workflows\Microsoft.Agents.AI.Workflows.csproj" />
|
||||
</ItemGroup>
|
||||
|
||||
</Project>
|
||||
@@ -0,0 +1,76 @@
|
||||
// Copyright (c) Microsoft. All rights reserved.
|
||||
|
||||
// This sample demonstrates evaluating a multi-agent workflow against a
|
||||
// golden answer using Foundry's reference-based Similarity evaluator.
|
||||
|
||||
using Azure.AI.Projects;
|
||||
using Azure.Identity;
|
||||
using Microsoft.Agents.AI;
|
||||
using Microsoft.Agents.AI.Workflows;
|
||||
using Microsoft.Extensions.AI;
|
||||
using FoundryEvals = Microsoft.Agents.AI.Foundry.FoundryEvals;
|
||||
|
||||
string endpoint = Environment.GetEnvironmentVariable("AZURE_AI_PROJECT_ENDPOINT")
|
||||
?? throw new InvalidOperationException("AZURE_AI_PROJECT_ENDPOINT is not set.");
|
||||
string deploymentName = Environment.GetEnvironmentVariable("AZURE_AI_MODEL_DEPLOYMENT_NAME") ?? "gpt-4o-mini";
|
||||
|
||||
// WARNING: DefaultAzureCredential is convenient for development but requires careful consideration in production.
|
||||
// In production, consider using a specific credential (e.g., ManagedIdentityCredential) to avoid
|
||||
// latency issues, unintended credential probing, and potential security risks from fallback mechanisms.
|
||||
AIProjectClient projectClient = new(new Uri(endpoint), new DefaultAzureCredential());
|
||||
|
||||
// Build a two-agent workflow: a researcher writes a draft answer, then an
|
||||
// editor polishes it into the final response that we compare to ground truth.
|
||||
// EmitAgentResponseEvents is enabled so the workflow surfaces an AgentResponseEvent
|
||||
// for each agent — this is what EvaluateAsync uses to find the overall final answer.
|
||||
var hostOptions = new AIAgentHostOptions { EmitAgentResponseEvents = true };
|
||||
|
||||
AIAgent researcher = projectClient.AsAIAgent(
|
||||
model: deploymentName,
|
||||
instructions: "You research questions and produce a short factual draft answer.",
|
||||
name: "researcher");
|
||||
|
||||
AIAgent editor = projectClient.AsAIAgent(
|
||||
model: deploymentName,
|
||||
instructions: "You take a draft answer and produce the final concise response.",
|
||||
name: "editor");
|
||||
|
||||
ExecutorBinding researcherExecutor = researcher.BindAsExecutor(hostOptions);
|
||||
ExecutorBinding editorExecutor = editor.BindAsExecutor(hostOptions);
|
||||
|
||||
Workflow workflow = new WorkflowBuilder(researcherExecutor)
|
||||
.AddEdge(researcherExecutor, editorExecutor)
|
||||
.Build();
|
||||
|
||||
// Run the workflow against the user question.
|
||||
const string Query = "What is the capital of France?";
|
||||
const string GroundTruth = "Paris";
|
||||
|
||||
await using Run run = await InProcessExecution.RunAsync(
|
||||
workflow,
|
||||
new ChatMessage(ChatRole.User, Query));
|
||||
|
||||
// Evaluate the overall workflow output against a golden answer using the
|
||||
// reference-based Similarity evaluator. The 'expectedOutput' value is stamped
|
||||
// onto the overall EvalItem.ExpectedOutput and is surfaced to Foundry as
|
||||
// `ground_truth` in the underlying JSONL payload.
|
||||
//
|
||||
// Per-agent breakdown is disabled here: ground truth applies to the workflow's
|
||||
// final answer, not to each sub-agent's intermediate output. Without
|
||||
// includePerAgent: false, the evaluator would be invoked for per-agent items
|
||||
// (which have no ExpectedOutput) and Similarity would fail validation.
|
||||
FoundryEvals similarity = new(projectClient, deploymentName, FoundryEvals.Similarity);
|
||||
|
||||
AgentEvaluationResults results = await run.EvaluateAsync(
|
||||
similarity,
|
||||
includePerAgent: false,
|
||||
expectedOutput: GroundTruth);
|
||||
|
||||
Console.WriteLine($"Query: {Query}");
|
||||
Console.WriteLine($"Expected: {GroundTruth}");
|
||||
Console.WriteLine($"Provider: {results.ProviderName}");
|
||||
Console.WriteLine($"Passed: {results.Passed}/{results.Total}");
|
||||
if (results.ReportUrl is not null)
|
||||
{
|
||||
Console.WriteLine($"Report: {results.ReportUrl}");
|
||||
}
|
||||
@@ -0,0 +1,37 @@
|
||||
# Evaluation - Workflow Expected Outputs
|
||||
|
||||
This sample demonstrates evaluating a multi-agent workflow's final answer
|
||||
against a golden expected output using Foundry's reference-based **Similarity**
|
||||
evaluator.
|
||||
|
||||
## What this sample demonstrates
|
||||
|
||||
- Building a small researcher → editor workflow
|
||||
- Running the workflow and obtaining a `Run`
|
||||
- Calling `run.EvaluateAsync(evaluator, expectedOutput: ...)` to attach a
|
||||
ground-truth answer to the overall workflow item
|
||||
- Using `FoundryEvals.Similarity`, which requires a `ground_truth` value
|
||||
per item
|
||||
|
||||
The `expectedOutput` value is stamped onto the overall `EvalItem.ExpectedOutput`
|
||||
and is surfaced to Foundry as `ground_truth` in the JSONL payload sent to the
|
||||
Evals API.
|
||||
|
||||
## Prerequisites
|
||||
|
||||
- .NET 10 SDK or later
|
||||
- Azure CLI installed and authenticated (`az login`)
|
||||
|
||||
Set the following environment variables:
|
||||
|
||||
```powershell
|
||||
$env:AZURE_AI_PROJECT_ENDPOINT="https://your-foundry-service.services.ai.azure.com/api/projects/your-foundry-project"
|
||||
$env:AZURE_AI_MODEL_DEPLOYMENT_NAME="gpt-4o-mini"
|
||||
```
|
||||
|
||||
## Run the sample
|
||||
|
||||
```powershell
|
||||
cd dotnet/samples/03-workflows/Evaluation
|
||||
dotnet run --project .\Evaluation_WorkflowExpectedOutputs
|
||||
```
|
||||
@@ -51,9 +51,7 @@ internal sealed class DevUIAuthFilter : IEndpointFilter
|
||||
|
||||
if (!isLoopback && !this._options.AllowRemoteAccess)
|
||||
{
|
||||
this._logger.LogWarning(
|
||||
"Rejected non-loopback DevUI request from {RemoteIp}. Set DevUIOptions.AllowRemoteAccess to permit remote callers.",
|
||||
remoteIp);
|
||||
DevUILog.RejectedNonLoopbackRequest(this._logger, remoteIp);
|
||||
return Results.Problem(
|
||||
statusCode: StatusCodes.Status403Forbidden,
|
||||
title: "DevUI access denied",
|
||||
|
||||
@@ -100,10 +100,7 @@ public static class DevUIExtensions
|
||||
|
||||
if (options.AllowRemoteAccess && !tokenConfigured && options.ConfigureEndpoints is null)
|
||||
{
|
||||
logger.LogWarning(
|
||||
"DevUI is configured with AllowRemoteAccess=true and no authentication. " +
|
||||
"Set DevUIOptions.AuthToken, the {EnvVar} environment variable, or attach an authorization policy via ConfigureEndpoints.",
|
||||
DevUIOptions.AuthTokenEnvironmentVariable);
|
||||
DevUILog.InsecurelyExposed(logger, DevUIOptions.AuthTokenEnvironmentVariable);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,20 @@
|
||||
// Copyright (c) Microsoft. All rights reserved.
|
||||
|
||||
using System.Net;
|
||||
|
||||
namespace Microsoft.Agents.AI.DevUI;
|
||||
|
||||
internal static partial class DevUILog
|
||||
{
|
||||
[LoggerMessage(
|
||||
EventId = 1,
|
||||
Level = LogLevel.Warning,
|
||||
Message = "Rejected non-loopback DevUI request from {RemoteIp}. Set DevUIOptions.AllowRemoteAccess to permit remote callers.")]
|
||||
public static partial void RejectedNonLoopbackRequest(ILogger logger, IPAddress? remoteIp);
|
||||
|
||||
[LoggerMessage(
|
||||
EventId = 2,
|
||||
Level = LogLevel.Warning,
|
||||
Message = "DevUI is configured with AllowRemoteAccess=true and no authentication. Set DevUIOptions.AuthToken, the {EnvVar} environment variable, or attach an authorization policy via ConfigureEndpoints.")]
|
||||
public static partial void InsecurelyExposed(ILogger logger, string envVar);
|
||||
}
|
||||
@@ -130,6 +130,7 @@ internal static class FoundryEvalConverter
|
||||
QueryMessages = ConvertMessages(queryMessages),
|
||||
ResponseMessages = ConvertMessages(responseMessages),
|
||||
Context = item.Context,
|
||||
GroundTruth = item.ExpectedOutput,
|
||||
ToolDefinitions = item.Tools is { Count: > 0 }
|
||||
? item.Tools
|
||||
.OfType<AIFunction>()
|
||||
@@ -185,6 +186,11 @@ internal static class FoundryEvalConverter
|
||||
dataMapping["context"] = "{{item.context}}";
|
||||
}
|
||||
|
||||
if (GroundTruthEvaluators.Contains(qualified))
|
||||
{
|
||||
dataMapping["ground_truth"] = "{{item.ground_truth}}";
|
||||
}
|
||||
|
||||
if (ToolEvaluators.Contains(qualified))
|
||||
{
|
||||
dataMapping["tool_definitions"] = "{{item.tool_definitions}}";
|
||||
@@ -206,7 +212,7 @@ internal static class FoundryEvalConverter
|
||||
/// <summary>
|
||||
/// Builds the <c>item_schema</c> for custom JSONL eval definitions.
|
||||
/// </summary>
|
||||
internal static WireItemSchema BuildItemSchema(bool hasContext = false, bool hasTools = false)
|
||||
internal static WireItemSchema BuildItemSchema(bool hasContext = false, bool hasTools = false, bool hasGroundTruth = false)
|
||||
{
|
||||
var properties = new Dictionary<string, WireSchemaProperty>
|
||||
{
|
||||
@@ -221,6 +227,11 @@ internal static class FoundryEvalConverter
|
||||
properties["context"] = new WireSchemaProperty { Type = "string" };
|
||||
}
|
||||
|
||||
if (hasGroundTruth)
|
||||
{
|
||||
properties["ground_truth"] = new WireSchemaProperty { Type = "string" };
|
||||
}
|
||||
|
||||
if (hasTools)
|
||||
{
|
||||
properties["tool_definitions"] = new WireSchemaProperty { Type = "array" };
|
||||
@@ -233,6 +244,31 @@ internal static class FoundryEvalConverter
|
||||
};
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Returns the subset of <paramref name="evaluators"/> that require a ground-truth
|
||||
/// (reference) value but cannot be evaluated because no item provided one.
|
||||
/// </summary>
|
||||
internal static List<string> FindMissingGroundTruthEvaluators(
|
||||
IEnumerable<string> evaluators,
|
||||
bool hasGroundTruth)
|
||||
{
|
||||
if (hasGroundTruth)
|
||||
{
|
||||
return [];
|
||||
}
|
||||
|
||||
var missing = new List<string>();
|
||||
foreach (var name in evaluators)
|
||||
{
|
||||
if (GroundTruthEvaluators.Contains(ResolveEvaluator(name)))
|
||||
{
|
||||
missing.Add(name);
|
||||
}
|
||||
}
|
||||
|
||||
return missing;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Resolves a short evaluator name to its fully-qualified <c>builtin.*</c> form.
|
||||
/// </summary>
|
||||
@@ -277,6 +313,12 @@ internal static class FoundryEvalConverter
|
||||
"builtin.tool_call_success",
|
||||
};
|
||||
|
||||
// Evaluators that require a ground_truth (reference) value per item.
|
||||
internal static readonly HashSet<string> GroundTruthEvaluators = new(StringComparer.OrdinalIgnoreCase)
|
||||
{
|
||||
"builtin.similarity",
|
||||
};
|
||||
|
||||
// Short name → fully-qualified name mapping.
|
||||
internal static readonly Dictionary<string, string> BuiltinEvaluators = new(StringComparer.OrdinalIgnoreCase)
|
||||
{
|
||||
|
||||
@@ -103,6 +103,9 @@ internal sealed class WireEvalItemPayload
|
||||
[JsonPropertyName("context")]
|
||||
public string? Context { get; init; }
|
||||
|
||||
[JsonPropertyName("ground_truth")]
|
||||
public string? GroundTruth { get; init; }
|
||||
|
||||
[JsonPropertyName("tool_definitions")]
|
||||
public List<WireToolDefinition>? ToolDefinitions { get; init; }
|
||||
}
|
||||
|
||||
@@ -145,6 +145,8 @@ public sealed class FoundryEvals : IAgentEvaluator
|
||||
|
||||
bool hasContext = payloads.Any(p => p.Context is not null);
|
||||
bool hasTools = payloads.Any(p => p.ToolDefinitions is { Count: > 0 });
|
||||
bool hasGroundTruth = payloads.Any(p => p.GroundTruth is not null);
|
||||
bool allHaveGroundTruth = payloads.Count > 0 && payloads.All(p => p.GroundTruth is not null);
|
||||
|
||||
// Filter out tool evaluators if no items have tools; auto-add ToolCallAccuracy if tools present
|
||||
var evaluators = FilterToolEvaluators(this._evaluatorNames, hasTools);
|
||||
@@ -153,13 +155,27 @@ public sealed class FoundryEvals : IAgentEvaluator
|
||||
evaluators = [.. evaluators, ToolCallAccuracy];
|
||||
}
|
||||
|
||||
// Fail fast if a ground-truth evaluator (e.g. similarity) is requested but not
|
||||
// every item carries an ExpectedOutput. Reference-based evaluators score each
|
||||
// item against its own ground truth, so even one missing value will surface as
|
||||
// a provider-side validation error. Catch it here with a clearer message.
|
||||
var missingGroundTruth = FoundryEvalConverter.FindMissingGroundTruthEvaluators(evaluators, allHaveGroundTruth);
|
||||
if (missingGroundTruth.Count > 0)
|
||||
{
|
||||
throw new InvalidOperationException(
|
||||
"The following evaluator(s) require a ground-truth/expected output on every item but " +
|
||||
$"at least one item is missing an {nameof(EvalItem.ExpectedOutput)}: {string.Join(", ", missingGroundTruth)}. " +
|
||||
"Provide an expected output per item (for example via the 'expectedOutput' parameter on EvaluateAsync), " +
|
||||
"or set 'includePerAgent: false' so the evaluator only runs on the overall item.");
|
||||
}
|
||||
|
||||
// 2. Create the evaluation definition
|
||||
var createEvalPayload = new WireCreateEvalRequest
|
||||
{
|
||||
Name = evalName,
|
||||
DataSourceConfig = new WireCustomDataSourceConfig
|
||||
{
|
||||
ItemSchema = FoundryEvalConverter.BuildItemSchema(hasContext, hasTools),
|
||||
ItemSchema = FoundryEvalConverter.BuildItemSchema(hasContext, hasTools, hasGroundTruth),
|
||||
},
|
||||
TestingCriteria = FoundryEvalConverter.BuildTestingCriteria(
|
||||
evaluators, this._model, includeDataMapping: true),
|
||||
@@ -822,15 +838,15 @@ public sealed class FoundryEvals : IAgentEvaluator
|
||||
var result = new EvalItemResult(itemId, status, scores);
|
||||
|
||||
// Extract error info from sample
|
||||
if (outputItem.TryGetProperty("sample", out var sample))
|
||||
if (outputItem.TryGetProperty("sample", out var sample) && sample.ValueKind == JsonValueKind.Object)
|
||||
{
|
||||
if (sample.TryGetProperty("error", out var errObj))
|
||||
if (sample.TryGetProperty("error", out var errObj) && errObj.ValueKind == JsonValueKind.Object)
|
||||
{
|
||||
result.ErrorCode = errObj.TryGetProperty("code", out var code) ? code.GetString() : null;
|
||||
result.ErrorMessage = errObj.TryGetProperty("message", out var msg) ? msg.GetString() : null;
|
||||
}
|
||||
|
||||
if (sample.TryGetProperty("usage", out var usage) && usage.TryGetProperty("total_tokens", out var tt) && tt.ValueKind == JsonValueKind.Number)
|
||||
if (sample.TryGetProperty("usage", out var usage) && usage.ValueKind == JsonValueKind.Object && usage.TryGetProperty("total_tokens", out var tt) && tt.ValueKind == JsonValueKind.Number)
|
||||
{
|
||||
var tokenUsage = new Dictionary<string, int>();
|
||||
if (usage.TryGetProperty("prompt_tokens", out var pt) && pt.ValueKind == JsonValueKind.Number)
|
||||
@@ -886,7 +902,7 @@ public sealed class FoundryEvals : IAgentEvaluator
|
||||
}
|
||||
|
||||
// Extract response_id from datasource_item
|
||||
if (outputItem.TryGetProperty("datasource_item", out var dsItem))
|
||||
if (outputItem.TryGetProperty("datasource_item", out var dsItem) && dsItem.ValueKind == JsonValueKind.Object)
|
||||
{
|
||||
if (dsItem.TryGetProperty("resp_id", out var respId))
|
||||
{
|
||||
|
||||
+111
-21
@@ -28,6 +28,17 @@ public static class WorkflowEvaluationExtensions
|
||||
/// Use <see cref="ConversationSplitters.LastTurn"/>, <see cref="ConversationSplitters.Full"/>,
|
||||
/// or a custom <see cref="IConversationSplitter"/> implementation.
|
||||
/// </param>
|
||||
/// <param name="expectedOutput">
|
||||
/// Optional ground-truth/expected output for the workflow's overall final answer.
|
||||
/// When provided, it is stamped onto the overall <see cref="EvalItem.ExpectedOutput"/>
|
||||
/// so reference-based evaluators (for example, similarity) can compare the
|
||||
/// workflow's response against a golden answer. Ground truth is only applied
|
||||
/// to the overall item; per-agent items are intentionally left without an
|
||||
/// expected output, since ground truth is defined against the final response.
|
||||
/// When using a reference-based evaluator that requires ground truth, set
|
||||
/// <paramref name="includePerAgent"/> to <see langword="false"/> to avoid
|
||||
/// invoking the evaluator on per-agent items that have no expected output.
|
||||
/// </param>
|
||||
/// <param name="cancellationToken">Cancellation token.</param>
|
||||
/// <returns>Evaluation results with optional per-agent sub-results.</returns>
|
||||
public static async Task<AgentEvaluationResults> EvaluateAsync(
|
||||
@@ -37,6 +48,7 @@ public static class WorkflowEvaluationExtensions
|
||||
bool includePerAgent = true,
|
||||
string evalName = "Workflow Eval",
|
||||
IConversationSplitter? splitter = null,
|
||||
string? expectedOutput = null,
|
||||
CancellationToken cancellationToken = default)
|
||||
{
|
||||
var events = run.OutgoingEvents.ToList();
|
||||
@@ -48,28 +60,26 @@ public static class WorkflowEvaluationExtensions
|
||||
var overallItems = new List<EvalItem>();
|
||||
if (includeOverall)
|
||||
{
|
||||
var finalResponse = events.OfType<AgentResponseEvent>().LastOrDefault();
|
||||
if (finalResponse is not null)
|
||||
var overallItem = BuildOverallItem(events, splitter, expectedOutput);
|
||||
if (overallItem is not null)
|
||||
{
|
||||
var firstInvoked = events.OfType<ExecutorInvokedEvent>().FirstOrDefault();
|
||||
var query = firstInvoked?.Data switch
|
||||
{
|
||||
ChatMessage cm => cm.Text ?? string.Empty,
|
||||
IReadOnlyList<ChatMessage> msgs => msgs.LastOrDefault(m => m.Role == ChatRole.User)?.Text ?? string.Empty,
|
||||
string s => s,
|
||||
_ => firstInvoked?.Data?.ToString() ?? string.Empty,
|
||||
};
|
||||
var conversation = new List<ChatMessage>
|
||||
{
|
||||
new(ChatRole.User, query),
|
||||
};
|
||||
|
||||
conversation.AddRange(finalResponse.Response.Messages);
|
||||
|
||||
overallItems.Add(new EvalItem(query, finalResponse.Response.Text, conversation)
|
||||
{
|
||||
Splitter = splitter,
|
||||
});
|
||||
overallItems.Add(overallItem);
|
||||
}
|
||||
else
|
||||
{
|
||||
// The caller asked for an overall evaluation but we couldn't find a final
|
||||
// response to score — almost always because the workflow's agents weren't
|
||||
// built with EmitAgentResponseEvents enabled (so no AgentResponseEvent was
|
||||
// emitted) and no terminal ExecutorCompletedEvent carried an AgentResponse
|
||||
// / ChatMessage / string payload. Fail loudly instead of silently returning
|
||||
// 0/0 (or skipping evaluation against a supplied expectedOutput).
|
||||
throw new InvalidOperationException(
|
||||
"Cannot evaluate the overall workflow output: no AgentResponseEvent or " +
|
||||
"ExecutorCompletedEvent with an AgentResponse/ChatMessage/string payload " +
|
||||
"was found in the run. Bind agents with " +
|
||||
"AIAgentHostOptions { EmitAgentResponseEvents = true } " +
|
||||
"(for example via agent.BindAsExecutor(new AIAgentHostOptions { EmitAgentResponseEvents = true })) " +
|
||||
"so the workflow surfaces the final agent response, or set 'includeOverall: false'.");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -97,6 +107,86 @@ public static class WorkflowEvaluationExtensions
|
||||
return overallResult;
|
||||
}
|
||||
|
||||
internal static EvalItem? BuildOverallItem(
|
||||
IReadOnlyList<WorkflowEvent> events,
|
||||
IConversationSplitter? splitter,
|
||||
string? expectedOutput)
|
||||
{
|
||||
var firstInvoked = events.OfType<ExecutorInvokedEvent>().FirstOrDefault();
|
||||
var query = firstInvoked?.Data switch
|
||||
{
|
||||
ChatMessage cm => cm.Text ?? string.Empty,
|
||||
IReadOnlyList<ChatMessage> msgs => msgs.LastOrDefault(m => m.Role == ChatRole.User)?.Text ?? string.Empty,
|
||||
string s => s,
|
||||
_ => firstInvoked?.Data?.ToString() ?? string.Empty,
|
||||
};
|
||||
|
||||
var conversation = new List<ChatMessage>
|
||||
{
|
||||
new(ChatRole.User, query),
|
||||
};
|
||||
|
||||
// Prefer AgentResponseEvent (only emitted when AIAgentHostOptions.EmitAgentResponseEvents
|
||||
// is enabled). Otherwise fall back to the last ExecutorCompletedEvent that carries an
|
||||
// AgentResponse / ChatMessage / string payload — these are always emitted by the runtime.
|
||||
var finalResponse = events.OfType<AgentResponseEvent>().LastOrDefault();
|
||||
string responseText;
|
||||
if (finalResponse is not null)
|
||||
{
|
||||
responseText = finalResponse.Response.Text;
|
||||
conversation.AddRange(finalResponse.Response.Messages);
|
||||
}
|
||||
else
|
||||
{
|
||||
ExecutorCompletedEvent? finalCompleted = null;
|
||||
for (int i = events.Count - 1; i >= 0; i--)
|
||||
{
|
||||
if (events[i] is ExecutorCompletedEvent completed
|
||||
&& !IsInternalExecutor(completed.ExecutorId)
|
||||
&& completed.Data is AgentResponse or ChatMessage or string)
|
||||
{
|
||||
finalCompleted = completed;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if (finalCompleted is null)
|
||||
{
|
||||
return null;
|
||||
}
|
||||
|
||||
switch (finalCompleted.Data)
|
||||
{
|
||||
case AgentResponse ar:
|
||||
responseText = ar.Text;
|
||||
conversation.AddRange(ar.Messages);
|
||||
break;
|
||||
case ChatMessage cm:
|
||||
responseText = cm.Text ?? string.Empty;
|
||||
conversation.Add(cm);
|
||||
break;
|
||||
case string s:
|
||||
responseText = s;
|
||||
conversation.Add(new ChatMessage(ChatRole.Assistant, s));
|
||||
break;
|
||||
default:
|
||||
// Unreachable — the for-loop above already constrains Data to one of the
|
||||
// three handled types. Throw if the contract drifts so the bug is visible
|
||||
// instead of silently dropping the overall item.
|
||||
throw new InvalidOperationException(
|
||||
"BuildOverallItem: unexpected ExecutorCompletedEvent.Data type " +
|
||||
$"'{finalCompleted.Data?.GetType().FullName ?? "null"}'. Expected " +
|
||||
$"{nameof(AgentResponse)}, {nameof(ChatMessage)}, or string.");
|
||||
}
|
||||
}
|
||||
|
||||
return new EvalItem(query, responseText, conversation)
|
||||
{
|
||||
Splitter = splitter,
|
||||
ExpectedOutput = expectedOutput,
|
||||
};
|
||||
}
|
||||
|
||||
internal static Dictionary<string, List<EvalItem>> ExtractAgentData(
|
||||
List<WorkflowEvent> events,
|
||||
IConversationSplitter? splitter)
|
||||
|
||||
@@ -179,6 +179,35 @@ public sealed class FoundryEvalConverterTests
|
||||
Assert.Null(payload.Context);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ConvertEvalItem_WithExpectedOutput_PopulatesGroundTruth()
|
||||
{
|
||||
// Arrange
|
||||
var item = new EvalItem(query: "q", response: "r")
|
||||
{
|
||||
ExpectedOutput = "the golden answer",
|
||||
};
|
||||
|
||||
// Act
|
||||
var payload = FoundryEvalConverter.ConvertEvalItem(item);
|
||||
|
||||
// Assert
|
||||
Assert.Equal("the golden answer", payload.GroundTruth);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ConvertEvalItem_WithoutExpectedOutput_OmitsGroundTruth()
|
||||
{
|
||||
// Arrange
|
||||
var item = new EvalItem(query: "q", response: "r");
|
||||
|
||||
// Act
|
||||
var payload = FoundryEvalConverter.ConvertEvalItem(item);
|
||||
|
||||
// Assert
|
||||
Assert.Null(payload.GroundTruth);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------
|
||||
// FoundryEvalConverter.BuildTestingCriteria tests
|
||||
// ---------------------------------------------------------------
|
||||
@@ -239,6 +268,33 @@ public sealed class FoundryEvalConverterTests
|
||||
Assert.Equal("{{item.context}}", mapping["context"]);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BuildTestingCriteria_SimilarityEvaluator_IncludesGroundTruth()
|
||||
{
|
||||
// Act
|
||||
var criteria = FoundryEvalConverter.BuildTestingCriteria(
|
||||
["similarity"], "gpt-4o-mini", includeDataMapping: true);
|
||||
|
||||
// Assert
|
||||
Assert.Single(criteria);
|
||||
Assert.Equal("builtin.similarity", criteria[0].EvaluatorName);
|
||||
var mapping = criteria[0].DataMapping;
|
||||
Assert.NotNull(mapping);
|
||||
Assert.True(mapping.ContainsKey("ground_truth"));
|
||||
Assert.Equal("{{item.ground_truth}}", mapping["ground_truth"]);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BuildTestingCriteria_NonGroundTruthEvaluator_OmitsGroundTruth()
|
||||
{
|
||||
var criteria = FoundryEvalConverter.BuildTestingCriteria(
|
||||
["relevance"], "gpt-4o-mini", includeDataMapping: true);
|
||||
|
||||
var mapping = criteria[0].DataMapping;
|
||||
Assert.NotNull(mapping);
|
||||
Assert.False(mapping.ContainsKey("ground_truth"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BuildTestingCriteria_WithoutDataMapping_OmitsMappingField()
|
||||
{
|
||||
@@ -282,6 +338,59 @@ public sealed class FoundryEvalConverterTests
|
||||
Assert.True(schema.Properties.ContainsKey("tool_definitions"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BuildItemSchema_WithGroundTruth_IncludesGroundTruthProperty()
|
||||
{
|
||||
// Act
|
||||
var schema = FoundryEvalConverter.BuildItemSchema(hasGroundTruth: true);
|
||||
|
||||
// Assert
|
||||
Assert.True(schema.Properties.ContainsKey("ground_truth"));
|
||||
Assert.Equal("string", schema.Properties["ground_truth"].Type);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BuildItemSchema_WithoutGroundTruth_OmitsGroundTruthProperty()
|
||||
{
|
||||
var schema = FoundryEvalConverter.BuildItemSchema();
|
||||
|
||||
Assert.False(schema.Properties.ContainsKey("ground_truth"));
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------
|
||||
// FoundryEvalConverter.FindMissingGroundTruthEvaluators tests
|
||||
// ---------------------------------------------------------------
|
||||
|
||||
[Fact]
|
||||
public void FindMissingGroundTruthEvaluators_NoGroundTruth_ReturnsSimilarity()
|
||||
{
|
||||
// Act
|
||||
var missing = FoundryEvalConverter.FindMissingGroundTruthEvaluators(
|
||||
["similarity", "relevance"], hasGroundTruth: false);
|
||||
|
||||
// Assert
|
||||
Assert.Single(missing);
|
||||
Assert.Equal("similarity", missing[0]);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void FindMissingGroundTruthEvaluators_HasGroundTruth_ReturnsEmpty()
|
||||
{
|
||||
var missing = FoundryEvalConverter.FindMissingGroundTruthEvaluators(
|
||||
["similarity"], hasGroundTruth: true);
|
||||
|
||||
Assert.Empty(missing);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void FindMissingGroundTruthEvaluators_NoGroundTruthEvaluators_ReturnsEmpty()
|
||||
{
|
||||
var missing = FoundryEvalConverter.FindMissingGroundTruthEvaluators(
|
||||
["relevance", "coherence"], hasGroundTruth: false);
|
||||
|
||||
Assert.Empty(missing);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------
|
||||
// FoundryEvalConverter.ConvertMessage DataContent test
|
||||
// ---------------------------------------------------------------
|
||||
|
||||
+9
-4
@@ -36,13 +36,18 @@ public sealed class InputWaiterTests : IDisposable
|
||||
{
|
||||
Task waitTask = this._waiter.WaitForInputAsync(TimeSpan.FromSeconds(5));
|
||||
|
||||
await Task.Delay(50);
|
||||
waitTask.IsCompleted.Should().BeFalse("the waiter should block until input is signaled");
|
||||
Task completedBeforeSignal = await Task.WhenAny(waitTask, Task.Delay(100));
|
||||
completedBeforeSignal.Should().NotBeSameAs(
|
||||
waitTask,
|
||||
"the waiter should not complete before input is signaled");
|
||||
|
||||
this._waiter.SignalInput();
|
||||
|
||||
Task completed = await Task.WhenAny(waitTask, Task.Delay(TimeSpan.FromSeconds(1)));
|
||||
completed.Should().BeSameAs(waitTask, "the wait task should complete after being signaled");
|
||||
Task completedAfterSignal = await Task.WhenAny(waitTask, Task.Delay(TimeSpan.FromSeconds(1)));
|
||||
completedAfterSignal.Should().BeSameAs(
|
||||
waitTask,
|
||||
"the wait task should complete after being signaled");
|
||||
|
||||
await waitTask;
|
||||
}
|
||||
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
// Copyright (c) Microsoft. All rights reserved.
|
||||
|
||||
using System;
|
||||
using System.Collections.Generic;
|
||||
using System.Threading.Tasks;
|
||||
using Microsoft.Extensions.AI;
|
||||
@@ -290,6 +291,121 @@ public sealed class WorkflowEvaluationTests
|
||||
Assert.DoesNotContain("end", result.Keys);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------
|
||||
// BuildOverallItem tests (expected output / ground truth)
|
||||
// ---------------------------------------------------------------
|
||||
|
||||
[Fact]
|
||||
public void BuildOverallItem_NoCompletedExecutorWithResponse_ReturnsNull()
|
||||
{
|
||||
// Arrange — no ExecutorCompletedEvent with usable response data and no AgentResponseEvent
|
||||
var events = new List<WorkflowEvent>
|
||||
{
|
||||
new ExecutorInvokedEvent("agent-1", "query"),
|
||||
};
|
||||
|
||||
// Act
|
||||
var item = WorkflowEvaluationExtensions.BuildOverallItem(events, splitter: null, expectedOutput: null);
|
||||
|
||||
// Assert
|
||||
Assert.Null(item);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BuildOverallItem_NoAgentResponseEvent_FallsBackToLastExecutorCompleted()
|
||||
{
|
||||
// Arrange — only ExecutorCompletedEvent (the default when EmitAgentResponseEvents is false)
|
||||
var finalResponse = new AgentResponse(new ChatMessage(ChatRole.Assistant, "Paris"));
|
||||
var events = new List<WorkflowEvent>
|
||||
{
|
||||
new ExecutorInvokedEvent("researcher", "What is the capital of France?"),
|
||||
new ExecutorCompletedEvent("researcher", new AgentResponse(new ChatMessage(ChatRole.Assistant, "draft"))),
|
||||
new ExecutorInvokedEvent("editor", "draft"),
|
||||
new ExecutorCompletedEvent("editor", finalResponse),
|
||||
};
|
||||
|
||||
// Act
|
||||
var item = WorkflowEvaluationExtensions.BuildOverallItem(
|
||||
events, splitter: null, expectedOutput: "Paris");
|
||||
|
||||
// Assert
|
||||
Assert.NotNull(item);
|
||||
Assert.Equal("What is the capital of France?", item.Query);
|
||||
Assert.Equal("Paris", item.Response);
|
||||
Assert.Equal("Paris", item.ExpectedOutput);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BuildOverallItem_WithFinalResponseAndExpectedOutput_StampsExpectedOutput()
|
||||
{
|
||||
// Arrange
|
||||
var finalResponse = new AgentResponse(new ChatMessage(ChatRole.Assistant, "Ofrece 41 planes"));
|
||||
var events = new List<WorkflowEvent>
|
||||
{
|
||||
new ExecutorInvokedEvent("agent-1", "How many plans does Netlife offer?"),
|
||||
new ExecutorCompletedEvent("agent-1", finalResponse),
|
||||
new AgentResponseEvent("agent-1", finalResponse),
|
||||
};
|
||||
|
||||
// Act
|
||||
var item = WorkflowEvaluationExtensions.BuildOverallItem(
|
||||
events, splitter: null, expectedOutput: "Ofrece 41 planes");
|
||||
|
||||
// Assert
|
||||
Assert.NotNull(item);
|
||||
Assert.Equal("How many plans does Netlife offer?", item.Query);
|
||||
Assert.Equal("Ofrece 41 planes", item.Response);
|
||||
Assert.Equal("Ofrece 41 planes", item.ExpectedOutput);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BuildOverallItem_WithFinalResponseAndNoExpectedOutput_LeavesExpectedOutputNull()
|
||||
{
|
||||
// Arrange
|
||||
var finalResponse = new AgentResponse(new ChatMessage(ChatRole.Assistant, "answer"));
|
||||
var events = new List<WorkflowEvent>
|
||||
{
|
||||
new ExecutorInvokedEvent("agent-1", "query"),
|
||||
new ExecutorCompletedEvent("agent-1", finalResponse),
|
||||
new AgentResponseEvent("agent-1", finalResponse),
|
||||
};
|
||||
|
||||
// Act
|
||||
var item = WorkflowEvaluationExtensions.BuildOverallItem(events, splitter: null, expectedOutput: null);
|
||||
|
||||
// Assert
|
||||
Assert.NotNull(item);
|
||||
Assert.Null(item.ExpectedOutput);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task EvaluateAsync_WithIncludeOverallButNoFinalResponse_ThrowsAsync()
|
||||
{
|
||||
// Arrange — build a workflow whose AIAgentHostExecutor is NOT bound with
|
||||
// EmitAgentResponseEvents=true, so no AgentResponseEvent is emitted, and the
|
||||
// ExecutorCompletedEvent for the host carries null Data. That is the scenario
|
||||
// where BuildOverallItem returns null. When the caller asks for an overall
|
||||
// evaluation (includeOverall: true), we should fail fast rather than silently
|
||||
// returning empty results — regardless of whether expectedOutput was supplied.
|
||||
var agent = new TestEchoAgent(name: "echo");
|
||||
var workflow = AgentWorkflowBuilder.BuildSequential(agent);
|
||||
var input = new List<ChatMessage> { new(ChatRole.User, "Hello") };
|
||||
|
||||
var evaluator = new LocalEvaluator(
|
||||
FunctionEvaluator.Create("noop", (EvalItem _) => true));
|
||||
|
||||
await using var run = await InProcessExecution.RunAsync(workflow, input);
|
||||
|
||||
// Act + Assert — throws even without expectedOutput
|
||||
var ex = await Assert.ThrowsAsync<InvalidOperationException>(() =>
|
||||
run.EvaluateAsync(
|
||||
evaluator,
|
||||
includeOverall: true,
|
||||
includePerAgent: false));
|
||||
|
||||
Assert.Contains("EmitAgentResponseEvents", ex.Message);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------
|
||||
// EvaluateAsync integration test
|
||||
// ---------------------------------------------------------------
|
||||
|
||||
@@ -69,7 +69,8 @@ agent_framework/
|
||||
|
||||
### Skills (`_skills.py`)
|
||||
|
||||
- **`Skill`** - A skill definition bundling instructions (`content`) with metadata, resources, and scripts. Supports `@skill.resource` and `@skill.script` decorators for adding components.
|
||||
- **`Skill`** - Abstract base for a skill definition bundling instructions (`content`) with frontmatter metadata, resources, and scripts. Concrete subclasses (`InlineSkill`, `FileSkill`, `ClassSkill`) accept a `frontmatter=SkillFrontmatter(...)` argument carrying the spec fields. Adding new spec fields is done in one place — on `SkillFrontmatter` — keeping the subclass constructors stable.
|
||||
- **`SkillFrontmatter`** - L1 discovery metadata for a skill (`name`, `description`, `license`, `compatibility`, `allowed_tools`, `metadata`). All fields are mutable plain attributes; the constructor validates `name`, `description`, and `compatibility` against the spec but post-construction assignments are not re-validated. Spec fields are reachable on every skill via `skill.frontmatter`.
|
||||
- **`SkillResource`** - Named supplementary content attached to a skill; holds either static `content` or a dynamic `function` (sync or async). Exactly one must be provided.
|
||||
- **`SkillScript`** - An executable script attached to a skill; holds either an inline `function` (code-defined, runs in-process) or a `path` to a file on disk (file-based, delegated to a runner). Exactly one must be provided.
|
||||
- **`SkillScriptRunner`** - Protocol for file-based script execution. Any callable matching `(skill, script, args) -> Any` satisfies it. Code-defined scripts do not use a runner.
|
||||
|
||||
@@ -147,6 +147,7 @@ from ._skills import (
|
||||
InlineSkillScript,
|
||||
InMemorySkillsSource,
|
||||
Skill,
|
||||
SkillFrontmatter,
|
||||
SkillResource,
|
||||
SkillScript,
|
||||
SkillScriptRunner,
|
||||
@@ -432,6 +433,7 @@ __all__ = [
|
||||
"SessionContext",
|
||||
"SingleEdgeGroup",
|
||||
"Skill",
|
||||
"SkillFrontmatter",
|
||||
"SkillResource",
|
||||
"SkillScript",
|
||||
"SkillScriptRunner",
|
||||
|
||||
@@ -10,7 +10,7 @@ import logging
|
||||
import re
|
||||
import sys
|
||||
from abc import abstractmethod
|
||||
from collections.abc import Callable, Collection, Sequence
|
||||
from collections.abc import Callable, Collection, Coroutine, Sequence
|
||||
from contextlib import AsyncExitStack, _AsyncGeneratorContextManager # type: ignore
|
||||
from datetime import timedelta
|
||||
from functools import partial
|
||||
@@ -264,6 +264,7 @@ class MCPTool:
|
||||
self.is_connected: bool = False
|
||||
self._tools_loaded: bool = False
|
||||
self._prompts_loaded: bool = False
|
||||
self._pending_reload_tasks: set[asyncio.Task[None]] = set()
|
||||
|
||||
def __str__(self) -> str:
|
||||
return f"MCPTool(name={self.name}, description={self.description})"
|
||||
@@ -905,12 +906,47 @@ class MCPTool:
|
||||
if isinstance(message, types.ServerNotification):
|
||||
match message.root.method:
|
||||
case "notifications/tools/list_changed":
|
||||
await self.load_tools()
|
||||
self._schedule_reload(self.load_tools())
|
||||
case "notifications/prompts/list_changed":
|
||||
await self.load_prompts()
|
||||
self._schedule_reload(self.load_prompts())
|
||||
case _:
|
||||
logger.debug("Unhandled notification: %s", message.root.method)
|
||||
|
||||
def _schedule_reload(self, coro: Coroutine[Any, Any, None]) -> None:
|
||||
"""Schedule a reload coroutine as a background task.
|
||||
|
||||
Reloads (load_tools / load_prompts) triggered by MCP server
|
||||
notifications must NOT be awaited inside the message handler because
|
||||
the handler runs on the MCP SDK's single-threaded receive loop.
|
||||
Awaiting a session request (e.g. ``list_tools``) from within that loop
|
||||
deadlocks: the receive loop cannot read the response while it is
|
||||
blocked waiting for the handler to return.
|
||||
|
||||
Instead we fire the reload as an independent ``asyncio.Task`` and keep
|
||||
a strong reference in ``_pending_reload_tasks`` so it is not garbage-
|
||||
collected before completion. Only one reload per kind (tools / prompts)
|
||||
is kept in flight; a new notification cancels the previous pending task
|
||||
for the same coroutine name to avoid unbounded growth.
|
||||
"""
|
||||
# Cancel-and-replace: only one reload per kind should be in flight.
|
||||
reload_name = f"mcp-reload:{self.name}:{coro.__qualname__}"
|
||||
for existing in list(self._pending_reload_tasks):
|
||||
if existing.get_name() == reload_name and not existing.done():
|
||||
logger.debug("Cancelling in-flight reload %s; superseded by new notification", reload_name)
|
||||
existing.cancel()
|
||||
|
||||
async def _safe_reload() -> None:
|
||||
try:
|
||||
await coro
|
||||
except asyncio.CancelledError:
|
||||
raise
|
||||
except Exception:
|
||||
logger.warning("Background MCP reload failed", exc_info=True)
|
||||
|
||||
task = asyncio.create_task(_safe_reload(), name=reload_name)
|
||||
self._pending_reload_tasks.add(task)
|
||||
task.add_done_callback(self._pending_reload_tasks.discard)
|
||||
|
||||
def _determine_approval_mode(
|
||||
self,
|
||||
*candidate_names: str,
|
||||
@@ -1047,6 +1083,14 @@ class MCPTool:
|
||||
params = types.PaginatedRequestParams(cursor=tool_list.nextCursor)
|
||||
|
||||
async def _close_on_owner(self) -> None:
|
||||
# Cancel any pending reload tasks before tearing down the session.
|
||||
tasks = list(self._pending_reload_tasks)
|
||||
for task in tasks:
|
||||
task.cancel()
|
||||
self._pending_reload_tasks.clear()
|
||||
if tasks:
|
||||
await asyncio.gather(*tasks, return_exceptions=True)
|
||||
|
||||
await self._safe_close_exit_stack()
|
||||
self._exit_stack = AsyncExitStack()
|
||||
self.session = None
|
||||
|
||||
@@ -465,41 +465,23 @@ class Skill(ABC):
|
||||
|
||||
A skill represents a domain-specific capability with instructions,
|
||||
resources, and scripts. Concrete implementations include
|
||||
:class:`FileSkill` (filesystem-backed) and :class:`InlineSkill`
|
||||
(code-defined).
|
||||
:class:`FileSkill` (filesystem-backed), :class:`InlineSkill`
|
||||
(code-defined), and :class:`ClassSkill` (class-based).
|
||||
|
||||
Skill metadata follows the
|
||||
`Agent Skills specification <https://agentskills.io/>`_.
|
||||
|
||||
Attributes:
|
||||
name: Skill name (lowercase letters, numbers, hyphens only).
|
||||
description: Human-readable description of the skill.
|
||||
Skill spec metadata (name, description, license, compatibility,
|
||||
allowed_tools, metadata) is exposed via the :attr:`frontmatter`
|
||||
property, which returns a :class:`SkillFrontmatter` instance.
|
||||
"""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
*,
|
||||
name: str,
|
||||
description: str,
|
||||
) -> None:
|
||||
"""Initialize a Skill.
|
||||
@property
|
||||
@abstractmethod
|
||||
def frontmatter(self) -> SkillFrontmatter:
|
||||
"""The L1 discovery metadata for this skill.
|
||||
|
||||
Validates the skill name and description against specification rules.
|
||||
|
||||
Args:
|
||||
name: Skill name (lowercase letters, numbers, hyphens only;
|
||||
max 64 characters; no leading/trailing/consecutive hyphens).
|
||||
description: Human-readable description of the skill
|
||||
(≤1024 characters).
|
||||
|
||||
Raises:
|
||||
ValueError: If the name or description is invalid.
|
||||
Contains the name, description, and other spec fields as defined by
|
||||
the `Agent Skills specification <https://agentskills.io/specification>`_.
|
||||
"""
|
||||
_validate_skill_name(name)
|
||||
_validate_skill_description(name, description)
|
||||
|
||||
self.name = name
|
||||
self.description = description
|
||||
...
|
||||
|
||||
@property
|
||||
@abstractmethod
|
||||
@@ -535,6 +517,68 @@ class Skill(ABC):
|
||||
return []
|
||||
|
||||
|
||||
@experimental(feature_id=ExperimentalFeature.SKILLS)
|
||||
class SkillFrontmatter:
|
||||
"""L1 discovery metadata for a :class:`Skill`.
|
||||
|
||||
Encapsulates all `Agent Skills specification <https://agentskills.io/specification>`_
|
||||
frontmatter fields in a single object. All fields are mutable plain
|
||||
attributes; callers may freely reassign them after construction.
|
||||
|
||||
The constructor validates ``name``, ``description``, and ``compatibility``
|
||||
against specification rules and raises :class:`ValueError` on invalid
|
||||
input. Assignments made after construction are **not** re-validated;
|
||||
callers are expected to honor the spec.
|
||||
|
||||
Attributes:
|
||||
name: Skill name (lowercase letters, numbers, hyphens only).
|
||||
description: Human-readable description of the skill.
|
||||
license: Optional license name or reference.
|
||||
compatibility: Optional compatibility information (≤500 characters).
|
||||
allowed_tools: Optional space-delimited pre-approved tool names.
|
||||
metadata: Optional arbitrary key-value pairs (shallow-copied on
|
||||
construction to avoid caller-owned dict aliasing).
|
||||
"""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
*,
|
||||
name: str,
|
||||
description: str,
|
||||
license: str | None = None,
|
||||
compatibility: str | None = None,
|
||||
allowed_tools: str | None = None,
|
||||
metadata: dict[str, str] | None = None,
|
||||
) -> None:
|
||||
"""Initialize a SkillFrontmatter.
|
||||
|
||||
Args:
|
||||
name: Skill name (lowercase letters, numbers, hyphens only;
|
||||
max 64 characters; no leading/trailing/consecutive hyphens).
|
||||
description: Human-readable description of the skill
|
||||
(≤1024 characters).
|
||||
license: Optional license name or reference.
|
||||
compatibility: Optional compatibility information
|
||||
(≤500 characters).
|
||||
allowed_tools: Optional space-delimited pre-approved tool names.
|
||||
metadata: Optional arbitrary key-value pairs.
|
||||
|
||||
Raises:
|
||||
ValueError: If the name, description, or compatibility is invalid.
|
||||
"""
|
||||
_validate_skill_name(name)
|
||||
_validate_skill_description(name, description)
|
||||
_validate_compatibility(compatibility)
|
||||
|
||||
self.name = name
|
||||
self.description = description
|
||||
self.compatibility = compatibility
|
||||
self.license = license
|
||||
self.allowed_tools = allowed_tools
|
||||
# Shallow-copy to avoid aliasing with caller-owned dict.
|
||||
self.metadata: dict[str, str] | None = dict(metadata) if metadata is not None else None
|
||||
|
||||
|
||||
def _validate_skill_name(name: str) -> None:
|
||||
"""Validate a skill name against specification rules.
|
||||
|
||||
@@ -573,6 +617,21 @@ def _validate_skill_description(name: str, description: str) -> None:
|
||||
)
|
||||
|
||||
|
||||
def _validate_compatibility(compatibility: str | None) -> None:
|
||||
"""Validate an optional compatibility value against specification rules.
|
||||
|
||||
Args:
|
||||
compatibility: The optional compatibility value to validate.
|
||||
|
||||
Raises:
|
||||
ValueError: If the value exceeds the maximum allowed length.
|
||||
"""
|
||||
if compatibility is not None and len(compatibility) > MAX_COMPATIBILITY_LENGTH:
|
||||
raise ValueError(
|
||||
f"Skill compatibility must be {MAX_COMPATIBILITY_LENGTH} characters or fewer."
|
||||
)
|
||||
|
||||
|
||||
def _build_skill_content(
|
||||
name: str,
|
||||
description: str,
|
||||
@@ -639,23 +698,17 @@ class InlineSkill(Skill):
|
||||
All resources and scripts should be configured before the skill is
|
||||
registered with a :class:`SkillsProvider`.
|
||||
|
||||
Attributes:
|
||||
name: Skill name (lowercase letters, numbers, hyphens only).
|
||||
description: Human-readable description of the skill.
|
||||
instructions: The skill instructions text.
|
||||
|
||||
Examples:
|
||||
With the decorator:
|
||||
|
||||
.. code-block:: python
|
||||
|
||||
skill = InlineSkill(
|
||||
name="db-skill",
|
||||
description="Database operations",
|
||||
frontmatter=SkillFrontmatter(
|
||||
name="db-skill",
|
||||
description="Database operations",
|
||||
),
|
||||
instructions="Use this skill for DB tasks.",
|
||||
)
|
||||
|
||||
|
||||
@skill.resource
|
||||
def get_schema() -> str:
|
||||
return "CREATE TABLE ..."
|
||||
@@ -664,8 +717,7 @@ class InlineSkill(Skill):
|
||||
def __init__(
|
||||
self,
|
||||
*,
|
||||
name: str,
|
||||
description: str,
|
||||
frontmatter: SkillFrontmatter,
|
||||
instructions: str,
|
||||
resources: Sequence[SkillResource] | None = None,
|
||||
scripts: Sequence[SkillScript] | None = None,
|
||||
@@ -673,19 +725,25 @@ class InlineSkill(Skill):
|
||||
"""Initialize an InlineSkill.
|
||||
|
||||
Args:
|
||||
name: Skill name (lowercase letters, numbers, hyphens only).
|
||||
description: Human-readable description of the skill (≤1024 chars).
|
||||
frontmatter: Skill specification metadata (name, description,
|
||||
and optional spec fields). Construct a :class:`SkillFrontmatter`
|
||||
with the desired fields.
|
||||
instructions: The skill instructions text.
|
||||
resources: Pre-built resources to attach to this skill.
|
||||
scripts: Pre-built scripts to attach to this skill.
|
||||
"""
|
||||
super().__init__(name=name, description=description)
|
||||
self._frontmatter = frontmatter
|
||||
|
||||
self.instructions = instructions
|
||||
self._resources: list[SkillResource] = list(resources) if resources is not None else []
|
||||
self._scripts: list[SkillScript] = list(scripts) if scripts is not None else []
|
||||
self._cached_content: str | None = None
|
||||
|
||||
@property
|
||||
def frontmatter(self) -> SkillFrontmatter:
|
||||
"""The L1 discovery metadata for this skill."""
|
||||
return self._frontmatter
|
||||
|
||||
@property
|
||||
def content(self) -> str:
|
||||
"""Synthesized XML content with name, description, instructions, resources, and scripts.
|
||||
@@ -697,7 +755,11 @@ class InlineSkill(Skill):
|
||||
return self._cached_content
|
||||
|
||||
self._cached_content = _build_skill_content(
|
||||
self.name, self.description, self.instructions, self._resources, self._scripts
|
||||
self._frontmatter.name,
|
||||
self._frontmatter.description,
|
||||
self.instructions,
|
||||
self._resources,
|
||||
self._scripts,
|
||||
)
|
||||
return self._cached_content
|
||||
|
||||
@@ -932,10 +994,6 @@ class ClassSkill(Skill, ABC):
|
||||
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):
|
||||
|
||||
@@ -944,8 +1002,10 @@ class ClassSkill(Skill, ABC):
|
||||
class UnitConverterSkill(ClassSkill):
|
||||
def __init__(self) -> None:
|
||||
super().__init__(
|
||||
name="unit-converter",
|
||||
description="Convert between common units.",
|
||||
frontmatter=SkillFrontmatter(
|
||||
name="unit-converter",
|
||||
description="Convert between common units.",
|
||||
),
|
||||
)
|
||||
|
||||
@property
|
||||
@@ -967,8 +1027,10 @@ class ClassSkill(Skill, ABC):
|
||||
class UnitConverterSkill(ClassSkill):
|
||||
def __init__(self) -> None:
|
||||
super().__init__(
|
||||
name="unit-converter",
|
||||
description="Convert between common units.",
|
||||
frontmatter=SkillFrontmatter(
|
||||
name="unit-converter",
|
||||
description="Convert between common units.",
|
||||
),
|
||||
)
|
||||
|
||||
@property
|
||||
@@ -989,22 +1051,25 @@ class ClassSkill(Skill, ABC):
|
||||
def __init__(
|
||||
self,
|
||||
*,
|
||||
name: str,
|
||||
description: str,
|
||||
frontmatter: SkillFrontmatter,
|
||||
) -> 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).
|
||||
frontmatter: Skill specification metadata (name, description,
|
||||
and optional spec fields). Construct a :class:`SkillFrontmatter`
|
||||
with the desired fields.
|
||||
"""
|
||||
super().__init__(name=name, description=description)
|
||||
self._frontmatter = frontmatter
|
||||
self._cached_content: str | None = None
|
||||
self._cached_resources: list[SkillResource] | None = None
|
||||
self._cached_scripts: list[SkillScript] | None = None
|
||||
|
||||
@property
|
||||
def frontmatter(self) -> SkillFrontmatter:
|
||||
"""The L1 discovery metadata for this skill."""
|
||||
return self._frontmatter
|
||||
|
||||
@staticmethod
|
||||
def resource(
|
||||
func: Callable[..., Any] | None = None,
|
||||
@@ -1152,7 +1217,7 @@ class ClassSkill(Skill, ABC):
|
||||
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}'. "
|
||||
f"Skill '{self._frontmatter.name}' already has a resource named '{resource_name}'. "
|
||||
"Ensure each @ClassSkill.resource has a unique name."
|
||||
)
|
||||
seen_names.add(resource_name)
|
||||
@@ -1212,7 +1277,7 @@ class ClassSkill(Skill, ABC):
|
||||
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}'. "
|
||||
f"Skill '{self._frontmatter.name}' already has a script named '{script_name}'. "
|
||||
"Ensure each @ClassSkill.script has a unique name."
|
||||
)
|
||||
seen_names.add(script_name)
|
||||
@@ -1240,7 +1305,11 @@ class ClassSkill(Skill, ABC):
|
||||
return self._cached_content
|
||||
|
||||
self._cached_content = _build_skill_content(
|
||||
self.name, self.description, self.instructions, self.resources, self.scripts
|
||||
self._frontmatter.name,
|
||||
self._frontmatter.description,
|
||||
self.instructions,
|
||||
self.resources,
|
||||
self.scripts,
|
||||
)
|
||||
return self._cached_content
|
||||
|
||||
@@ -1250,16 +1319,13 @@ class FileSkill(Skill):
|
||||
"""A :class:`Skill` discovered from a filesystem directory backed by a SKILL.md file.
|
||||
|
||||
Attributes:
|
||||
name: Skill name (lowercase letters, numbers, hyphens only).
|
||||
description: Human-readable description of the skill.
|
||||
path: Absolute path to the directory containing this skill.
|
||||
"""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
*,
|
||||
name: str,
|
||||
description: str,
|
||||
frontmatter: SkillFrontmatter,
|
||||
content: str,
|
||||
path: str,
|
||||
resources: Sequence[SkillResource] | None = None,
|
||||
@@ -1268,20 +1334,26 @@ class FileSkill(Skill):
|
||||
"""Initialize a FileSkill.
|
||||
|
||||
Args:
|
||||
name: Skill name (lowercase letters, numbers, hyphens only).
|
||||
description: Human-readable description of the skill (≤1024 chars).
|
||||
frontmatter: Skill specification metadata parsed from the
|
||||
SKILL.md file's YAML frontmatter (name, description,
|
||||
and optional spec fields).
|
||||
content: The full raw SKILL.md file content including YAML frontmatter.
|
||||
path: Absolute path to the skill directory on disk.
|
||||
resources: Resources discovered for this skill.
|
||||
scripts: Scripts discovered for this skill.
|
||||
"""
|
||||
super().__init__(name=name, description=description)
|
||||
self._frontmatter = frontmatter
|
||||
|
||||
self._content = content
|
||||
self.path = path
|
||||
self._resources: list[SkillResource] = list(resources) if resources is not None else []
|
||||
self._scripts: list[SkillScript] = list(scripts) if scripts is not None else []
|
||||
|
||||
@property
|
||||
def frontmatter(self) -> SkillFrontmatter:
|
||||
"""The L1 discovery metadata for this skill."""
|
||||
return self._frontmatter
|
||||
|
||||
@property
|
||||
def content(self) -> str:
|
||||
"""The skill content provided at construction time."""
|
||||
@@ -1346,6 +1418,7 @@ SKILL_FILE_NAME: Final[str] = "SKILL.md"
|
||||
MAX_SEARCH_DEPTH: Final[int] = 2
|
||||
MAX_NAME_LENGTH: Final[int] = 64
|
||||
MAX_DESCRIPTION_LENGTH: Final[int] = 1024
|
||||
MAX_COMPATIBILITY_LENGTH: Final[int] = 500
|
||||
DEFAULT_RESOURCE_EXTENSIONS: Final[tuple[str, ...]] = (
|
||||
".md",
|
||||
".json",
|
||||
@@ -1366,10 +1439,24 @@ FRONTMATTER_RE = re.compile(
|
||||
re.MULTILINE | re.DOTALL,
|
||||
)
|
||||
|
||||
# Matches YAML "key: value" lines. Group 1 = key, Group 2 = quoted value,
|
||||
# Group 3 = unquoted value.
|
||||
# Matches top-level YAML "key: value" lines (unindented). Group 1 = key,
|
||||
# Group 2 = quoted value, Group 3 = unquoted value. Only matches keys at
|
||||
# column 0 so that indented children (e.g. under "metadata:") are not
|
||||
# mistakenly captured as top-level fields.
|
||||
YAML_KV_RE = re.compile(
|
||||
r"^\s*(\w+)\s*:\s*(?:[\"'](.+?)[\"']|(.+?))\s*$",
|
||||
r"^([\w-]+)\s*:\s*(?:[\"'](.+?)[\"']|(.+?))\s*$",
|
||||
re.MULTILINE,
|
||||
)
|
||||
|
||||
# Matches a YAML "metadata:" block followed by indented key-value pairs.
|
||||
YAML_METADATA_BLOCK_RE = re.compile(
|
||||
r"^metadata\s*:\s*$\n((?:[ \t]+\S.*\n?)+)",
|
||||
re.MULTILINE,
|
||||
)
|
||||
|
||||
# Matches indented "key: value" lines within a metadata block.
|
||||
YAML_INDENTED_KV_RE = re.compile(
|
||||
r"^\s+([\w-]+)\s*:\s*(?:[\"'](.+?)[\"']|(.+?))\s*$",
|
||||
re.MULTILINE,
|
||||
)
|
||||
|
||||
@@ -1377,6 +1464,7 @@ YAML_KV_RE = re.compile(
|
||||
# must not start or end with a hyphen, and must not contain consecutive hyphens.
|
||||
VALID_NAME_RE = re.compile(r"^[a-z0-9]([a-z0-9]*-[a-z0-9])*[a-z0-9]*$")
|
||||
|
||||
|
||||
# Default system prompt template for advertising available skills to the model.
|
||||
# Use {skills} as the placeholder for the generated skills XML list.
|
||||
DEFAULT_SKILLS_INSTRUCTION_PROMPT = """\
|
||||
@@ -1463,7 +1551,7 @@ class SkillsProvider(ContextProvider):
|
||||
FileSkillsSource("./skills", script_runner=my_runner),
|
||||
InMemorySkillsSource([my_code_skill]),
|
||||
]),
|
||||
predicate=lambda s: s.name != "internal",
|
||||
predicate=lambda s: s.frontmatter.name != "internal",
|
||||
)
|
||||
)
|
||||
provider = SkillsProvider(source)
|
||||
@@ -1698,10 +1786,10 @@ class SkillsProvider(ContextProvider):
|
||||
|
||||
lines: list[str] = []
|
||||
# Sort by name for deterministic output
|
||||
for skill in sorted(skills, key=lambda s: s.name):
|
||||
for skill in sorted(skills, key=lambda s: s.frontmatter.name):
|
||||
lines.append(" <skill>")
|
||||
lines.append(f" <name>{xml_escape(skill.name)}</name>")
|
||||
lines.append(f" <description>{xml_escape(skill.description)}</description>")
|
||||
lines.append(f" <name>{xml_escape(skill.frontmatter.name)}</name>")
|
||||
lines.append(f" <description>{xml_escape(skill.frontmatter.description)}</description>")
|
||||
lines.append(" </skill>")
|
||||
|
||||
return template.format(
|
||||
@@ -1920,7 +2008,7 @@ class SkillsProvider(ContextProvider):
|
||||
def _find_skill(skills: Sequence[Skill], name: str) -> Skill | None:
|
||||
"""Find a skill by name (case-insensitive linear scan)."""
|
||||
name_lower = name.lower()
|
||||
return next((s for s in skills if s.name.lower() == name_lower), None)
|
||||
return next((s for s in skills if s.frontmatter.name.lower() == name_lower), None)
|
||||
|
||||
def _load_skill(self, skills: Sequence[Skill], skill_name: str) -> str:
|
||||
"""Return the full content for the named skill.
|
||||
@@ -2179,19 +2267,18 @@ class FileSkillsSource(SkillsSource):
|
||||
if parsed is None:
|
||||
continue
|
||||
|
||||
name, description, content = parsed
|
||||
frontmatter, content = parsed
|
||||
|
||||
if name in skills:
|
||||
if frontmatter.name in skills:
|
||||
logger.warning(
|
||||
"Duplicate skill name '%s': skill from '%s' skipped in favor of existing skill",
|
||||
name,
|
||||
frontmatter.name,
|
||||
skill_path,
|
||||
)
|
||||
continue
|
||||
|
||||
file_skill = FileSkill(
|
||||
name=name,
|
||||
description=description,
|
||||
frontmatter=frontmatter,
|
||||
content=content,
|
||||
path=skill_path,
|
||||
)
|
||||
@@ -2208,8 +2295,8 @@ class FileSkillsSource(SkillsSource):
|
||||
FileSkillScript(name=sn, full_path=script_full_path, runner=self._script_runner)
|
||||
)
|
||||
|
||||
skills[file_skill.name] = file_skill
|
||||
logger.info("Loaded skill: %s", file_skill.name)
|
||||
skills[file_skill.frontmatter.name] = file_skill
|
||||
logger.info("Loaded skill: %s", file_skill.frontmatter.name)
|
||||
|
||||
logger.info("Successfully loaded %d skills", len(skills))
|
||||
return list(skills.values())
|
||||
@@ -2438,8 +2525,9 @@ class FileSkillsSource(SkillsSource):
|
||||
name: str | None,
|
||||
description: str | None,
|
||||
source: str,
|
||||
compatibility: str | None = None,
|
||||
) -> str | None:
|
||||
"""Validate a skill's name and description against naming rules.
|
||||
"""Validate a skill's name, description, and compatibility against naming rules.
|
||||
|
||||
Enforces length limits, character-set restrictions, and non-emptiness
|
||||
for both file-based and code-defined skills.
|
||||
@@ -2449,6 +2537,7 @@ class FileSkillsSource(SkillsSource):
|
||||
description: Skill description to validate.
|
||||
source: Human-readable label for diagnostics (e.g. a file path
|
||||
or ``"code skill"``).
|
||||
compatibility: Optional compatibility value to validate.
|
||||
|
||||
Returns:
|
||||
A diagnostic error string if validation fails, or ``None`` if valid.
|
||||
@@ -2472,24 +2561,32 @@ class FileSkillsSource(SkillsSource):
|
||||
f"Must be {MAX_DESCRIPTION_LENGTH} characters or fewer."
|
||||
)
|
||||
|
||||
if compatibility is not None and len(compatibility) > MAX_COMPATIBILITY_LENGTH:
|
||||
return (
|
||||
f"Skill '{name}' from '{source}' has an invalid compatibility: "
|
||||
f"Must be {MAX_COMPATIBILITY_LENGTH} characters or fewer."
|
||||
)
|
||||
|
||||
return None
|
||||
|
||||
@staticmethod
|
||||
def _extract_frontmatter(
|
||||
content: str,
|
||||
skill_file_path: str,
|
||||
) -> tuple[str, str] | None:
|
||||
) -> SkillFrontmatter | None:
|
||||
"""Extract and validate YAML frontmatter from a SKILL.md file.
|
||||
|
||||
Parses the ``---``-delimited frontmatter block for ``name`` and
|
||||
``description`` fields.
|
||||
Parses the ``---``-delimited frontmatter block for all
|
||||
`agentskills.io specification <https://agentskills.io/specification>`_
|
||||
fields: ``name``, ``description``, ``license``, ``compatibility``,
|
||||
``allowed-tools``, and ``metadata``.
|
||||
|
||||
Args:
|
||||
content: Raw text content of the SKILL.md file.
|
||||
skill_file_path: Path to the file (used in diagnostic messages only).
|
||||
|
||||
Returns:
|
||||
A ``(name, description)`` tuple on success, or ``None`` if the
|
||||
A :class:`SkillFrontmatter` on success, or ``None`` if the
|
||||
frontmatter is missing, malformed, or fails validation.
|
||||
"""
|
||||
match = FRONTMATTER_RE.search(content)
|
||||
@@ -2500,35 +2597,63 @@ class FileSkillsSource(SkillsSource):
|
||||
yaml_content = match.group(1).strip()
|
||||
name: str | None = None
|
||||
description: str | None = None
|
||||
license_value: str | None = None
|
||||
compatibility: str | None = None
|
||||
allowed_tools: str | None = None
|
||||
|
||||
for kv_match in YAML_KV_RE.finditer(yaml_content):
|
||||
key = kv_match.group(1)
|
||||
value = kv_match.group(2) if kv_match.group(2) is not None else kv_match.group(3)
|
||||
|
||||
if key.lower() == "name":
|
||||
key_lower = key.lower()
|
||||
if key_lower == "name":
|
||||
name = value
|
||||
elif key.lower() == "description":
|
||||
elif key_lower == "description":
|
||||
description = value
|
||||
elif key_lower == "license":
|
||||
license_value = value
|
||||
elif key_lower == "compatibility":
|
||||
compatibility = value
|
||||
elif key_lower == "allowed-tools":
|
||||
allowed_tools = value
|
||||
|
||||
error = FileSkillsSource._validate_skill_metadata(name, description, skill_file_path)
|
||||
# Parse metadata block (indented key-value pairs under "metadata:").
|
||||
metadata: dict[str, str] | None = None
|
||||
metadata_match = YAML_METADATA_BLOCK_RE.search(yaml_content)
|
||||
if metadata_match:
|
||||
metadata = {}
|
||||
for kv_match in YAML_INDENTED_KV_RE.finditer(metadata_match.group(1)):
|
||||
mk = kv_match.group(1)
|
||||
mv = kv_match.group(2) if kv_match.group(2) is not None else kv_match.group(3)
|
||||
metadata[mk] = mv
|
||||
|
||||
error = FileSkillsSource._validate_skill_metadata(name, description, skill_file_path, compatibility)
|
||||
if error:
|
||||
logger.error(error)
|
||||
return None
|
||||
|
||||
# name and description are guaranteed non-None after validation
|
||||
return name, description # type: ignore[return-value]
|
||||
# name and description are guaranteed non-None after validation;
|
||||
# SkillFrontmatter re-validates as a defense-in-depth invariant.
|
||||
return SkillFrontmatter(
|
||||
name=cast(str, name),
|
||||
description=cast(str, description),
|
||||
license=license_value,
|
||||
compatibility=compatibility,
|
||||
allowed_tools=allowed_tools,
|
||||
metadata=metadata,
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
def _read_and_parse_skill_file(
|
||||
skill_dir_path: str,
|
||||
) -> tuple[str, str, str] | None:
|
||||
) -> tuple[SkillFrontmatter, str] | None:
|
||||
"""Read and parse the SKILL.md file in *skill_dir_path*.
|
||||
|
||||
Args:
|
||||
skill_dir_path: Absolute path to the directory containing ``SKILL.md``.
|
||||
|
||||
Returns:
|
||||
A ``(name, description, content)`` tuple where *content* is the
|
||||
A ``(frontmatter, content)`` tuple where *content* is the
|
||||
full raw file text, or ``None`` if the file cannot be read or
|
||||
its frontmatter is invalid.
|
||||
"""
|
||||
@@ -2540,23 +2665,21 @@ class FileSkillsSource(SkillsSource):
|
||||
logger.error("Failed to read SKILL.md at '%s'", skill_file)
|
||||
return None
|
||||
|
||||
result = FileSkillsSource._extract_frontmatter(content, str(skill_file))
|
||||
if result is None:
|
||||
frontmatter = FileSkillsSource._extract_frontmatter(content, str(skill_file))
|
||||
if frontmatter is None:
|
||||
return None
|
||||
|
||||
name, description = result
|
||||
|
||||
dir_name = Path(skill_dir_path).name
|
||||
if name != dir_name:
|
||||
if frontmatter.name != dir_name:
|
||||
logger.error(
|
||||
"SKILL.md at '%s' has frontmatter name '%s' that does not match the directory name '%s'; skipping.",
|
||||
skill_file,
|
||||
name,
|
||||
frontmatter.name,
|
||||
dir_name,
|
||||
)
|
||||
return None
|
||||
|
||||
return name, description, content
|
||||
return frontmatter, content
|
||||
|
||||
@staticmethod
|
||||
def _discover_skill_directories(skill_paths: Sequence[str]) -> list[str]:
|
||||
@@ -2704,12 +2827,12 @@ class DeduplicatingSkillsSource(DelegatingSkillsSource):
|
||||
result: list[Skill] = []
|
||||
|
||||
for skill in skills:
|
||||
key = skill.name.lower()
|
||||
key = skill.frontmatter.name.lower()
|
||||
if key in seen:
|
||||
logger.warning(
|
||||
"Duplicate skill name '%s': skill skipped in favor of existing skill '%s'",
|
||||
skill.name,
|
||||
seen[key].name,
|
||||
skill.frontmatter.name,
|
||||
seen[key].frontmatter.name,
|
||||
)
|
||||
continue
|
||||
seen[key] = skill
|
||||
@@ -2730,7 +2853,7 @@ class FilteringSkillsSource(DelegatingSkillsSource):
|
||||
|
||||
filtered = FilteringSkillsSource(
|
||||
inner_source=my_source,
|
||||
predicate=lambda s: s.name != "internal",
|
||||
predicate=lambda s: s.frontmatter.name != "internal",
|
||||
)
|
||||
skills = await filtered.get_skills()
|
||||
"""
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
# Copyright (c) Microsoft. All rights reserved.
|
||||
# type: ignore[reportPrivateUsage]
|
||||
import asyncio
|
||||
import contextlib
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
@@ -1615,7 +1616,7 @@ async def test_mcp_connection_reset_integration():
|
||||
|
||||
async def test_mcp_tool_message_handler_notification():
|
||||
"""Test that message_handler correctly processes tools/list_changed and prompts/list_changed
|
||||
notifications."""
|
||||
notifications by scheduling reloads as background tasks."""
|
||||
tool = MCPStdioTool(name="test_tool", command="python")
|
||||
|
||||
# Mock the load_tools and load_prompts methods
|
||||
@@ -1629,6 +1630,8 @@ async def test_mcp_tool_message_handler_notification():
|
||||
|
||||
result = await tool.message_handler(tools_notification)
|
||||
assert result is None
|
||||
# The reload is scheduled as a background task; let it run.
|
||||
await asyncio.sleep(0)
|
||||
tool.load_tools.assert_called_once()
|
||||
|
||||
# Reset mock
|
||||
@@ -1641,6 +1644,7 @@ async def test_mcp_tool_message_handler_notification():
|
||||
|
||||
result = await tool.message_handler(prompts_notification)
|
||||
assert result is None
|
||||
await asyncio.sleep(0)
|
||||
tool.load_prompts.assert_called_once()
|
||||
|
||||
# Test unhandled notification
|
||||
@@ -1664,6 +1668,112 @@ async def test_mcp_tool_message_handler_error():
|
||||
assert result is None
|
||||
|
||||
|
||||
async def test_mcp_tool_message_handler_does_not_block_receive_loop():
|
||||
"""Test that message_handler does not deadlock the MCP receive loop.
|
||||
|
||||
Regression test for https://github.com/microsoft/agent-framework/issues/4828.
|
||||
When the MCP server sends a ``notifications/tools/list_changed``
|
||||
notification, the handler must NOT await ``load_tools()`` synchronously
|
||||
because that would block the single-threaded MCP receive loop, preventing
|
||||
it from delivering the ``list_tools`` response — a classic deadlock.
|
||||
"""
|
||||
tool = MCPStdioTool(name="test_tool", command="python")
|
||||
|
||||
# Use an event to make load_tools block until we release it.
|
||||
# This simulates load_tools waiting for a session response that the
|
||||
# receive loop would need to deliver.
|
||||
release = asyncio.Event()
|
||||
|
||||
async def slow_load_tools():
|
||||
await release.wait()
|
||||
|
||||
tool.load_tools = slow_load_tools # type: ignore[assignment]
|
||||
|
||||
tools_notification = Mock(spec=types.ServerNotification)
|
||||
tools_notification.root = Mock()
|
||||
tools_notification.root.method = "notifications/tools/list_changed"
|
||||
|
||||
# message_handler must return immediately even though load_tools blocks.
|
||||
await tool.message_handler(tools_notification)
|
||||
|
||||
# If the handler had awaited load_tools synchronously, we would never
|
||||
# reach this line (deadlock). Verify the reload task is pending.
|
||||
assert len(tool._pending_reload_tasks) == 1
|
||||
|
||||
# Unblock the reload so the background task finishes cleanly.
|
||||
release.set()
|
||||
# Wait for the pending reload task(s) to complete so their done-callbacks
|
||||
# have a chance to remove them from _pending_reload_tasks.
|
||||
await asyncio.wait_for(asyncio.gather(*tool._pending_reload_tasks), timeout=1)
|
||||
assert len(tool._pending_reload_tasks) == 0
|
||||
|
||||
|
||||
async def test_mcp_tool_message_handler_reload_failure_is_logged(caplog: pytest.LogCaptureFixture):
|
||||
"""Background reload errors are logged, not raised into the receive loop."""
|
||||
tool = MCPStdioTool(name="test_tool", command="python")
|
||||
tool.load_tools = AsyncMock(side_effect=RuntimeError("connection lost"))
|
||||
|
||||
tools_notification = Mock(spec=types.ServerNotification)
|
||||
tools_notification.root = Mock()
|
||||
tools_notification.root.method = "notifications/tools/list_changed"
|
||||
|
||||
await tool.message_handler(tools_notification)
|
||||
# Let the background task run — it should not propagate the exception.
|
||||
# Snapshot tasks and await them to ensure done-callbacks fire.
|
||||
pending = list(tool._pending_reload_tasks)
|
||||
if pending:
|
||||
await asyncio.wait_for(asyncio.gather(*pending, return_exceptions=True), timeout=1)
|
||||
tool.load_tools.assert_called_once()
|
||||
assert len(tool._pending_reload_tasks) == 0
|
||||
|
||||
# Verify the warning was actually logged with exception info.
|
||||
reload_warnings = [r for r in caplog.records if "Background MCP reload failed" in r.message]
|
||||
assert len(reload_warnings) == 1
|
||||
assert reload_warnings[0].levelname == "WARNING"
|
||||
assert reload_warnings[0].exc_info is not None
|
||||
|
||||
|
||||
async def test_mcp_tool_message_handler_cancel_and_replace():
|
||||
"""Sending two notifications in quick succession cancels the first reload task."""
|
||||
tool = MCPStdioTool(name="test_tool", command="python")
|
||||
|
||||
release = asyncio.Event()
|
||||
call_count = 0
|
||||
|
||||
async def blocking_load_tools():
|
||||
nonlocal call_count
|
||||
call_count += 1
|
||||
await release.wait()
|
||||
|
||||
tool.load_tools = blocking_load_tools # type: ignore[assignment]
|
||||
|
||||
notification = Mock(spec=types.ServerNotification)
|
||||
notification.root = Mock()
|
||||
notification.root.method = "notifications/tools/list_changed"
|
||||
|
||||
# First notification — starts a blocking reload task.
|
||||
await tool.message_handler(notification)
|
||||
assert len(tool._pending_reload_tasks) == 1
|
||||
first_task = next(iter(tool._pending_reload_tasks))
|
||||
|
||||
# Second notification — should cancel the first and replace it.
|
||||
await tool.message_handler(notification)
|
||||
# Yield to the event loop so the cancellation is processed.
|
||||
with contextlib.suppress(asyncio.CancelledError):
|
||||
await first_task
|
||||
|
||||
assert first_task.cancelled()
|
||||
|
||||
assert len(tool._pending_reload_tasks) == 1
|
||||
second_task = next(iter(tool._pending_reload_tasks))
|
||||
assert second_task is not first_task
|
||||
|
||||
# Unblock and let the second task finish.
|
||||
release.set()
|
||||
await asyncio.wait_for(asyncio.gather(*tool._pending_reload_tasks), timeout=1)
|
||||
assert len(tool._pending_reload_tasks) == 0
|
||||
|
||||
|
||||
async def test_mcp_tool_sampling_callback_no_client():
|
||||
"""Test sampling callback error path when no chat client is available."""
|
||||
tool = MCPStdioTool(name="test_tool", command="python")
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -429,7 +429,12 @@ class _FullHistoryReplayCoordinator(Executor):
|
||||
|
||||
|
||||
@pytest.mark.xfail(
|
||||
reason="reset_service_session support not yet implemented — see #4047",
|
||||
reason=(
|
||||
"Tracks the executor-layer half of #3295: AgentExecutor should clear service_session_id "
|
||||
"when handed a full prior conversation. The wire-level 'Duplicate item' API error is "
|
||||
"already closed by the chat-client strip in #3295; this xfail covers the defense-in-depth "
|
||||
"follow-up that makes the executor wiring reflect intent."
|
||||
),
|
||||
strict=True,
|
||||
)
|
||||
async def test_run_request_with_full_history_clears_service_session_id() -> None:
|
||||
|
||||
@@ -96,7 +96,7 @@ def serve(
|
||||
ui_enabled: bool = True,
|
||||
instrumentation_enabled: bool = False,
|
||||
mode: str = "developer",
|
||||
auth_enabled: bool = False,
|
||||
auth_enabled: bool = True,
|
||||
auth_token: str | None = None,
|
||||
) -> None:
|
||||
"""Launch Agent Framework DevUI with simple API.
|
||||
@@ -126,52 +126,29 @@ def serve(
|
||||
if not isinstance(port, int) or not (1 <= port <= 65535):
|
||||
raise ValueError(f"Invalid port: {port}. Must be integer between 1 and 65535")
|
||||
|
||||
# Security check: Warn if network-exposed without authentication
|
||||
# Security check: warn loudly when network-exposed without authentication.
|
||||
if host not in ("127.0.0.1", "localhost") and not auth_enabled:
|
||||
logger.warning("⚠️ WARNING: Exposing DevUI to network without authentication!")
|
||||
logger.warning("⚠️ This is INSECURE - anyone on your network can access your agents")
|
||||
logger.warning("💡 For network exposure, add --auth flag: devui --host 0.0.0.0 --auth")
|
||||
logger.warning("WARNING: Exposing DevUI to the network with --no-auth.")
|
||||
logger.warning("Anyone on your network can read agent metadata and trigger requests.")
|
||||
logger.warning("Drop --no-auth and DevUI will require Bearer tokens.")
|
||||
|
||||
# Handle authentication configuration
|
||||
if auth_enabled:
|
||||
# Refuse to auto-generate a token for network-exposed binds. Auto-generated tokens
|
||||
# are fine for localhost convenience; for anything else, require an explicit token.
|
||||
if auth_enabled and not auth_token:
|
||||
import os
|
||||
import secrets
|
||||
|
||||
# Check if token is in environment variable first
|
||||
if not auth_token:
|
||||
auth_token = os.environ.get("DEVUI_AUTH_TOKEN")
|
||||
|
||||
# Auto-generate token if STILL not provided
|
||||
if not auth_token:
|
||||
# Check if we're in a production-like environment
|
||||
env_token = os.environ.get("DEVUI_AUTH_TOKEN")
|
||||
if not env_token:
|
||||
is_production = (
|
||||
host not in ("127.0.0.1", "localhost") # Exposed to network
|
||||
or os.environ.get("CI") == "true" # Running in CI
|
||||
or os.environ.get("KUBERNETES_SERVICE_HOST") # Running in k8s
|
||||
host not in ("127.0.0.1", "localhost")
|
||||
or os.environ.get("CI") == "true"
|
||||
or os.environ.get("KUBERNETES_SERVICE_HOST")
|
||||
)
|
||||
|
||||
if is_production:
|
||||
# REFUSE to start without explicit token
|
||||
logger.error("❌ Authentication enabled but no token provided")
|
||||
logger.error("❌ Auto-generated tokens are NOT secure for network-exposed deployments")
|
||||
logger.error("💡 Set token: export DEVUI_AUTH_TOKEN=<your-secure-token>")
|
||||
logger.error("💡 Or pass: serve(entities=[...], auth_token='your-token')")
|
||||
logger.error("Authentication required but no token provided.")
|
||||
logger.error("Set DEVUI_AUTH_TOKEN env var or pass auth_token='...' to serve().")
|
||||
raise ValueError("DEVUI_AUTH_TOKEN required when host is not localhost")
|
||||
|
||||
# Development mode: auto-generate and show
|
||||
auth_token = secrets.token_urlsafe(32)
|
||||
logger.info("🔒 Authentication enabled with auto-generated token")
|
||||
logger.info("\n" + "=" * 70)
|
||||
logger.info("🔑 DEV TOKEN (localhost only, shown once):")
|
||||
logger.info(f" {auth_token}")
|
||||
logger.info("=" * 70 + "\n")
|
||||
else:
|
||||
logger.info("🔒 Authentication enabled with provided token")
|
||||
|
||||
# Set environment variable for server to use
|
||||
os.environ["AUTH_REQUIRED"] = "true"
|
||||
os.environ["DEVUI_AUTH_TOKEN"] = auth_token
|
||||
|
||||
# Enable instrumentation if requested
|
||||
if instrumentation_enabled:
|
||||
from agent_framework.observability import enable_instrumentation
|
||||
@@ -187,6 +164,8 @@ def serve(
|
||||
cors_origins=cors_origins,
|
||||
ui_enabled=ui_enabled,
|
||||
mode=mode,
|
||||
auth_enabled=auth_enabled,
|
||||
auth_token=auth_token,
|
||||
)
|
||||
|
||||
# Register in-memory entities if provided
|
||||
|
||||
@@ -79,15 +79,15 @@ Examples:
|
||||
)
|
||||
|
||||
parser.add_argument(
|
||||
"--auth",
|
||||
"--no-auth",
|
||||
action="store_true",
|
||||
help="Enable authentication via Bearer token (required for deployed environments)",
|
||||
help="Disable Bearer token authentication. DevUI is auth-enabled by default; use this to opt out.",
|
||||
)
|
||||
|
||||
parser.add_argument(
|
||||
"--auth-token",
|
||||
type=str,
|
||||
help="Custom authentication token (auto-generated if not provided with --auth)",
|
||||
help="Custom Bearer token. Auto-generated and logged at startup when omitted.",
|
||||
)
|
||||
|
||||
parser.add_argument("--version", action="version", version=f"Agent Framework DevUI {get_version()}")
|
||||
@@ -184,7 +184,7 @@ def main() -> None:
|
||||
ui_enabled=ui_enabled,
|
||||
instrumentation_enabled=args.instrumentation,
|
||||
mode=mode,
|
||||
auth_enabled=args.auth,
|
||||
auth_enabled=not args.no_auth,
|
||||
auth_token=args.auth_token, # Pass through explicit token only
|
||||
)
|
||||
|
||||
|
||||
@@ -75,6 +75,8 @@ class DevServer:
|
||||
cors_origins: list[str] | None = None,
|
||||
ui_enabled: bool = True,
|
||||
mode: str = "developer",
|
||||
auth_enabled: bool = True,
|
||||
auth_token: str | None = None,
|
||||
) -> None:
|
||||
"""Initialize the development server.
|
||||
|
||||
@@ -85,20 +87,26 @@ class DevServer:
|
||||
cors_origins: List of allowed CORS origins
|
||||
ui_enabled: Whether to enable the UI
|
||||
mode: Server mode - 'developer' (full access, verbose errors) or 'user' (restricted APIs, generic errors)
|
||||
auth_enabled: Whether to require Bearer token auth on /v1/* endpoints. Defaults to True.
|
||||
auth_token: Bearer token. If None and auth_enabled, falls back to the DEVUI_AUTH_TOKEN
|
||||
environment variable, then to an auto-generated token (logged at startup).
|
||||
"""
|
||||
self.entities_dir = entities_dir
|
||||
self.port = port
|
||||
self.host = host
|
||||
|
||||
# Smart CORS defaults: permissive for localhost, restrictive for network-exposed deployments
|
||||
# CORS default is same-origin only (empty allowlist) on every host. The
|
||||
# previous wildcard-on-localhost default let any webpage the developer
|
||||
# visited read DevUI's responses cross-origin. Callers who need a real
|
||||
# cross-origin dev frontend pass an explicit allowlist.
|
||||
if cors_origins is None:
|
||||
# Localhost development: allow cross-origin for dev tools (e.g., frontend dev server)
|
||||
# Network-exposed: empty list (same-origin only, no CORS)
|
||||
cors_origins = ["*"] if host in ("127.0.0.1", "localhost") else []
|
||||
cors_origins = []
|
||||
|
||||
self.cors_origins = cors_origins
|
||||
self.ui_enabled = ui_enabled
|
||||
self.mode = mode
|
||||
self.auth_enabled = auth_enabled
|
||||
self.auth_token = self._resolve_auth_token(auth_enabled, auth_token)
|
||||
self.executor: AgentFrameworkExecutor | None = None
|
||||
self.openai_executor: OpenAIExecutor | None = None
|
||||
self.deployment_manager = DeploymentManager()
|
||||
@@ -110,6 +118,37 @@ class DevServer:
|
||||
"""Set in-memory entities to register on startup."""
|
||||
self._pending_entities = entities
|
||||
|
||||
_LOOPBACK_HOSTS = frozenset({"127.0.0.1", "localhost", "[::1]", "::1"})
|
||||
|
||||
def _loopback_allowed_hosts(self) -> frozenset[str] | None:
|
||||
"""Return the Host-header allowlist when bound to a loopback interface, else None.
|
||||
|
||||
Returning None disables Host-header enforcement (e.g. for 0.0.0.0 / public binds,
|
||||
where the operator is intentionally exposing the service).
|
||||
"""
|
||||
host = self.host.lower()
|
||||
if host not in self._LOOPBACK_HOSTS:
|
||||
return None
|
||||
return self._LOOPBACK_HOSTS
|
||||
|
||||
@staticmethod
|
||||
def _resolve_auth_token(auth_enabled: bool, auth_token: str | None) -> str | None:
|
||||
"""Resolve the active Bearer token. Returns None when auth is disabled."""
|
||||
if not auth_enabled:
|
||||
return None
|
||||
if auth_token:
|
||||
return auth_token
|
||||
env_token = os.getenv("DEVUI_AUTH_TOKEN")
|
||||
if env_token:
|
||||
return env_token
|
||||
generated = secrets.token_urlsafe(32)
|
||||
logger.info("=" * 70)
|
||||
logger.info("DevUI authentication enabled with auto-generated token:")
|
||||
logger.info(f" {generated}")
|
||||
logger.info("Pass it as: Authorization: Bearer <token>")
|
||||
logger.info("=" * 70)
|
||||
return generated
|
||||
|
||||
def _is_dev_mode(self) -> bool:
|
||||
"""Check if running in developer mode.
|
||||
|
||||
@@ -336,6 +375,11 @@ class DevServer:
|
||||
lifespan=lifespan,
|
||||
)
|
||||
|
||||
# Middleware registration order matters: Starlette wraps later-added
|
||||
# middleware around earlier-added ones, so the LAST registered runs
|
||||
# outermost (sees the request first). We want Host-header enforcement
|
||||
# to run before CORS/auth, so it is registered last below.
|
||||
|
||||
# Add CORS middleware
|
||||
# Note: allow_credentials cannot be True when allow_origins is ["*"]
|
||||
# For localhost dev with wildcard origins, credentials are disabled
|
||||
@@ -350,29 +394,24 @@ class DevServer:
|
||||
allow_headers=["*"],
|
||||
)
|
||||
|
||||
# Add authentication middleware using decorator pattern
|
||||
# Auth is enabled by presence of DEVUI_AUTH_TOKEN
|
||||
auth_token = os.getenv("DEVUI_AUTH_TOKEN", "")
|
||||
auth_required = bool(auth_token)
|
||||
|
||||
if auth_required:
|
||||
# Bearer-token authentication. Enabled by default; opt out via
|
||||
# DevServer(auth_enabled=False) for embedded/test scenarios.
|
||||
if self.auth_enabled and self.auth_token:
|
||||
expected_token = self.auth_token
|
||||
logger.info("Authentication middleware enabled")
|
||||
|
||||
@app.middleware("http")
|
||||
async def auth_middleware(request: Request, call_next: Callable[[Request], Awaitable[Any]]) -> Any:
|
||||
"""Validate Bearer token authentication.
|
||||
|
||||
Skips authentication for health, meta, static UI endpoints, and OPTIONS requests.
|
||||
Skips authentication for health, the UI shell, static assets, and OPTIONS preflight.
|
||||
"""
|
||||
# Skip auth for OPTIONS (CORS preflight) requests
|
||||
if request.method == "OPTIONS":
|
||||
return await call_next(request)
|
||||
|
||||
# Skip auth for health checks, meta endpoint, and static files
|
||||
if request.url.path in ["/health", "/meta", "/"] or request.url.path.startswith("/assets"):
|
||||
if request.url.path in ["/health", "/"] or request.url.path.startswith("/assets"):
|
||||
return await call_next(request)
|
||||
|
||||
# Check Authorization header
|
||||
auth_header = request.headers.get("Authorization")
|
||||
if not auth_header or not auth_header.startswith("Bearer "):
|
||||
return JSONResponse(
|
||||
@@ -388,9 +427,8 @@ class DevServer:
|
||||
},
|
||||
)
|
||||
|
||||
# Extract and validate token
|
||||
token = auth_header.replace("Bearer ", "", 1).strip()
|
||||
if not secrets.compare_digest(token, auth_token):
|
||||
if not secrets.compare_digest(token, expected_token):
|
||||
return JSONResponse(
|
||||
status_code=401,
|
||||
content={
|
||||
@@ -402,11 +440,40 @@ class DevServer:
|
||||
},
|
||||
)
|
||||
|
||||
# Token valid, proceed
|
||||
return await call_next(request)
|
||||
|
||||
_ = auth_middleware
|
||||
|
||||
# Host-header allowlist for loopback binds: on a loopback interface, only
|
||||
# accept requests whose Host header names a loopback address. Registered LAST
|
||||
# so it runs outermost, rejecting non-loopback Host values before CORS/auth
|
||||
# (and before CORS can short-circuit a preflight on a rebound request).
|
||||
allowed_hosts = self._loopback_allowed_hosts()
|
||||
if allowed_hosts is not None:
|
||||
expected_hosts = allowed_hosts
|
||||
|
||||
@app.middleware("http")
|
||||
async def host_header_middleware(request: Request, call_next: Callable[[Request], Awaitable[Any]]) -> Any:
|
||||
host_header = request.headers.get("host", "")
|
||||
hostname = host_header.split(":", 1)[0].lower()
|
||||
if hostname and hostname not in expected_hosts:
|
||||
return JSONResponse(
|
||||
status_code=400,
|
||||
content={
|
||||
"error": {
|
||||
"message": (
|
||||
f"Invalid Host header '{host_header}'. DevUI is bound to a "
|
||||
"loopback interface and only accepts requests addressed to it."
|
||||
),
|
||||
"type": "invalid_host",
|
||||
"code": "host_not_allowed",
|
||||
}
|
||||
},
|
||||
)
|
||||
return await call_next(request)
|
||||
|
||||
_ = host_header_middleware
|
||||
|
||||
self._register_routes(app)
|
||||
self._mount_ui(app)
|
||||
|
||||
@@ -427,8 +494,6 @@ class DevServer:
|
||||
@app.get("/meta", response_model=MetaResponse)
|
||||
async def get_meta() -> MetaResponse:
|
||||
"""Get server metadata and configuration."""
|
||||
import os
|
||||
|
||||
# Ensure executors are initialized to check capabilities
|
||||
openai_executor = await self._ensure_openai_executor()
|
||||
|
||||
@@ -442,7 +507,7 @@ class DevServer:
|
||||
"openai_proxy": openai_executor.is_configured,
|
||||
"deployment": True, # Deployment feature is available
|
||||
},
|
||||
auth_required=bool(os.getenv("DEVUI_AUTH_TOKEN")),
|
||||
auth_required=self.auth_enabled,
|
||||
)
|
||||
|
||||
@app.get("/v1/entities", response_model=DiscoveryResponse)
|
||||
@@ -750,7 +815,6 @@ class DevServer:
|
||||
headers={
|
||||
"Cache-Control": "no-cache",
|
||||
"Connection": "keep-alive",
|
||||
"Access-Control-Allow-Origin": "*",
|
||||
},
|
||||
)
|
||||
return await openai_executor.execute_sync(request)
|
||||
@@ -794,7 +858,6 @@ class DevServer:
|
||||
headers={
|
||||
"Cache-Control": "no-cache",
|
||||
"Connection": "keep-alive",
|
||||
"Access-Control-Allow-Origin": "*",
|
||||
"X-Response-ID": response_id, # Include ID for debugging/tracking
|
||||
},
|
||||
)
|
||||
|
||||
@@ -3,11 +3,15 @@
|
||||
"""Focused tests for server functionality."""
|
||||
|
||||
import asyncio
|
||||
import inspect
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
from conftest import MockAgent
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
import agent_framework_devui
|
||||
from agent_framework_devui import DevServer
|
||||
from agent_framework_devui._utils import extract_executor_message_types, select_primary_input_type
|
||||
from agent_framework_devui.models._openai_custom import AgentFrameworkRequest
|
||||
@@ -99,11 +103,11 @@ async def test_server_execution_streaming(test_entities_dir):
|
||||
|
||||
def test_configuration():
|
||||
"""Test basic configuration."""
|
||||
server = DevServer(entities_dir="test", port=9000, host="localhost")
|
||||
server = DevServer(entities_dir="test", port=9000, host="localhost", auth_enabled=False)
|
||||
assert server.port == 9000
|
||||
assert server.host == "localhost"
|
||||
assert server.entities_dir == "test"
|
||||
assert server.cors_origins == ["*"]
|
||||
assert server.cors_origins == []
|
||||
assert server.ui_enabled
|
||||
|
||||
|
||||
@@ -252,15 +256,18 @@ async def test_api_restrictions_in_user_mode():
|
||||
"""Test that developer APIs are restricted in user mode."""
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
# Create servers with different modes
|
||||
dev_server = DevServer(mode="developer")
|
||||
user_server = DevServer(mode="user")
|
||||
# Create servers with different modes. auth_enabled=False isolates this test
|
||||
# to mode behavior — auth has its own dedicated suite.
|
||||
dev_server = DevServer(mode="developer", auth_enabled=False)
|
||||
user_server = DevServer(mode="user", auth_enabled=False)
|
||||
|
||||
dev_app = dev_server.create_app()
|
||||
user_app = user_server.create_app()
|
||||
|
||||
dev_client = TestClient(dev_app)
|
||||
user_client = TestClient(user_app)
|
||||
# base_url sets the Host header to a loopback alias so the loopback
|
||||
# host-header allowlist accepts the request.
|
||||
dev_client = TestClient(dev_app, base_url="http://127.0.0.1")
|
||||
user_client = TestClient(user_app, base_url="http://127.0.0.1")
|
||||
|
||||
# Test 1: Health endpoint should work in both modes
|
||||
assert dev_client.get("/health").status_code == 200
|
||||
@@ -403,3 +410,171 @@ async def test_checkpoint_api_endpoints(test_entities_dir):
|
||||
# Test delete non-existent checkpoint
|
||||
deleted = await storage.delete("nonexistent")
|
||||
assert deleted is False
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# Security posture: default CORS, auth, host-header, and streaming headers.
|
||||
# =============================================================================
|
||||
|
||||
|
||||
def _server_with_mock_agent(**kwargs) -> DevServer:
|
||||
"""Build a DevServer with one in-memory mock agent registered."""
|
||||
server = DevServer(**kwargs)
|
||||
server.set_pending_entities([MockAgent(id="mock", name="Mock", response_text="hi")])
|
||||
return server
|
||||
|
||||
|
||||
def test_streaming_response_does_not_hardcode_acao_header():
|
||||
"""A streaming /v1/responses must not set Access-Control-Allow-Origin itself.
|
||||
|
||||
The endpoint previously hardcoded `Access-Control-Allow-Origin: *` on the
|
||||
StreamingResponse, bypassing CORSMiddleware. With no Origin header on the
|
||||
request, CORSMiddleware never adds ACAO — so any ACAO we see proves the
|
||||
streaming handler is still setting it.
|
||||
"""
|
||||
server = _server_with_mock_agent(auth_token="s3cret")
|
||||
app = server.get_app()
|
||||
|
||||
with TestClient(app, base_url="http://127.0.0.1") as client:
|
||||
response = client.post(
|
||||
"/v1/responses",
|
||||
json={"metadata": {"entity_id": "mock"}, "input": "hello", "stream": True},
|
||||
headers={"Authorization": "Bearer s3cret"},
|
||||
)
|
||||
|
||||
assert "access-control-allow-origin" not in {k.lower() for k in response.headers}, (
|
||||
"Streaming response sets ACAO directly, bypassing CORSMiddleware"
|
||||
)
|
||||
|
||||
|
||||
def test_cors_default_does_not_allow_arbitrary_origin_even_on_localhost():
|
||||
"""Default CORS must not echo Access-Control-Allow-Origin to arbitrary origins.
|
||||
|
||||
Previous default was `["*"]` on localhost binds, which let any webpage the
|
||||
developer visited read DevUI's responses. Default is now `[]` — opt in by
|
||||
passing `cors_origins=[...]` explicitly.
|
||||
"""
|
||||
server = _server_with_mock_agent(host="127.0.0.1", auth_token="s3cret")
|
||||
app = server.get_app()
|
||||
|
||||
with TestClient(app, base_url="http://127.0.0.1") as client:
|
||||
preflight = client.options(
|
||||
"/v1/entities",
|
||||
headers={
|
||||
"Origin": "https://evil.example",
|
||||
"Access-Control-Request-Method": "GET",
|
||||
},
|
||||
)
|
||||
assert preflight.headers.get("access-control-allow-origin") not in ("*", "https://evil.example")
|
||||
|
||||
actual = client.get(
|
||||
"/v1/entities",
|
||||
headers={"Origin": "https://evil.example", "Authorization": "Bearer s3cret"},
|
||||
)
|
||||
assert actual.headers.get("access-control-allow-origin") not in ("*", "https://evil.example")
|
||||
|
||||
|
||||
def test_devserver_requires_auth_by_default(monkeypatch):
|
||||
"""A bare DevServer() must reject unauthenticated /v1/* requests.
|
||||
|
||||
Previously auth was opt-in via DEVUI_AUTH_TOKEN env var; the new default is
|
||||
auth-on so a bare `devui ./agents` invocation does not expose an open API.
|
||||
"""
|
||||
monkeypatch.delenv("DEVUI_AUTH_TOKEN", raising=False)
|
||||
|
||||
server = DevServer()
|
||||
app = server.get_app()
|
||||
|
||||
with TestClient(app, base_url="http://127.0.0.1") as client:
|
||||
response = client.get("/v1/entities")
|
||||
|
||||
assert response.status_code == 401
|
||||
|
||||
|
||||
def test_devserver_auth_can_be_explicitly_disabled(monkeypatch):
|
||||
"""Callers can opt out of auth with auth_enabled=False (escape hatch for tests / trusted hosts)."""
|
||||
monkeypatch.delenv("DEVUI_AUTH_TOKEN", raising=False)
|
||||
|
||||
server = _server_with_mock_agent(auth_enabled=False)
|
||||
app = server.get_app()
|
||||
|
||||
with TestClient(app, base_url="http://127.0.0.1") as client:
|
||||
response = client.get("/v1/entities")
|
||||
|
||||
assert response.status_code == 200
|
||||
|
||||
|
||||
def test_devserver_accepts_request_with_valid_bearer_token(monkeypatch):
|
||||
"""When auth is on, supplying the configured Bearer token grants access."""
|
||||
monkeypatch.delenv("DEVUI_AUTH_TOKEN", raising=False)
|
||||
|
||||
server = DevServer(auth_token="s3cret")
|
||||
app = server.get_app()
|
||||
|
||||
with TestClient(app, base_url="http://127.0.0.1") as client:
|
||||
response = client.get("/v1/entities", headers={"Authorization": "Bearer s3cret"})
|
||||
|
||||
assert response.status_code == 200
|
||||
|
||||
|
||||
def test_meta_endpoint_requires_auth(monkeypatch):
|
||||
"""/meta exposes capability flags (deployment, instrumentation, version) — gate it behind auth.
|
||||
|
||||
Previously /meta was in the auth-bypass list alongside /health and /, so any
|
||||
unauthenticated caller could read the deployment's capability flags.
|
||||
"""
|
||||
monkeypatch.delenv("DEVUI_AUTH_TOKEN", raising=False)
|
||||
|
||||
server = DevServer(auth_token="s3cret")
|
||||
app = server.get_app()
|
||||
|
||||
with TestClient(app, base_url="http://127.0.0.1") as client:
|
||||
unauth = client.get("/meta")
|
||||
assert unauth.status_code == 401
|
||||
|
||||
ok = client.get("/meta", headers={"Authorization": "Bearer s3cret"})
|
||||
assert ok.status_code == 200
|
||||
|
||||
|
||||
def test_loopback_bind_rejects_non_allowlisted_host_header(monkeypatch):
|
||||
"""A loopback-bound server must reject requests with a non-loopback Host header.
|
||||
|
||||
On a loopback bind, only Host values that name a loopback address are valid;
|
||||
anything else (e.g. an external hostname that happens to resolve to 127.0.0.1)
|
||||
is rejected before any handler runs.
|
||||
"""
|
||||
monkeypatch.delenv("DEVUI_AUTH_TOKEN", raising=False)
|
||||
|
||||
server = DevServer(host="127.0.0.1", auth_enabled=False)
|
||||
app = server.get_app()
|
||||
|
||||
with TestClient(app, base_url="http://127.0.0.1") as client:
|
||||
rebound = client.get("/health", headers={"Host": "evil.example"})
|
||||
assert rebound.status_code == 400
|
||||
|
||||
ok = client.get("/health", headers={"Host": "127.0.0.1"})
|
||||
assert ok.status_code == 200
|
||||
|
||||
ok_localhost = client.get("/health", headers={"Host": "localhost:8080"})
|
||||
assert ok_localhost.status_code == 200
|
||||
|
||||
|
||||
def test_serve_defaults_to_auth_enabled():
|
||||
"""`serve()`'s public signature must default to auth_enabled=True."""
|
||||
sig = inspect.signature(agent_framework_devui.serve)
|
||||
assert sig.parameters["auth_enabled"].default is True, (
|
||||
"serve() must default to auth_enabled=True so `devui ./agents` is secure out of the box"
|
||||
)
|
||||
|
||||
|
||||
def test_cli_enables_auth_by_default_and_supports_no_auth_optout():
|
||||
"""`devui ./agents` must produce auth-enabled config; `--no-auth` is the explicit escape hatch."""
|
||||
from agent_framework_devui._cli import create_cli_parser
|
||||
|
||||
parser = create_cli_parser()
|
||||
|
||||
default_args = parser.parse_args([])
|
||||
assert default_args.no_auth is False, "Default CLI invocation should leave auth on"
|
||||
|
||||
optout_args = parser.parse_args(["--no-auth"])
|
||||
assert optout_args.no_auth is True
|
||||
|
||||
@@ -582,7 +582,7 @@ def test_sample_peak_renderer_rss_mb_uses_browser_process_tree(
|
||||
def memory_regression_server() -> Generator[tuple[str, str]]:
|
||||
"""Start DevUI with a synthetic streaming agent and yield the base URL plus entity ID."""
|
||||
|
||||
server = DevServer(host="127.0.0.1", port=0)
|
||||
server = DevServer(host="127.0.0.1", port=0, auth_enabled=False)
|
||||
server.register_entities([
|
||||
MemoryStressAgent(
|
||||
id="memory-stream-agent",
|
||||
|
||||
@@ -435,7 +435,7 @@ async def test_chat_message_parsing_with_function_calls() -> None:
|
||||
Message(role="tool", contents=[function_result]),
|
||||
]
|
||||
|
||||
prepared_messages = client._prepare_messages_for_openai(messages)
|
||||
prepared_messages = client._prepare_messages_for_openai(messages, request_uses_service_side_storage=False)
|
||||
|
||||
assert prepared_messages == [
|
||||
{
|
||||
|
||||
@@ -1409,29 +1409,31 @@ class RawOpenAIChatClient( # type: ignore[misc]
|
||||
}
|
||||
additional_properties = message.additional_properties
|
||||
replays_local_storage = "_attribution" in additional_properties
|
||||
uses_service_side_storage = request_uses_service_side_storage and not replays_local_storage
|
||||
# Reasoning items are only valid in input when they directly preceded a function_call
|
||||
# in the same response. Including a reasoning item that preceded a text response
|
||||
# (i.e. no function_call in the same message) causes an API error:
|
||||
# "reasoning was provided without its required following item."
|
||||
#
|
||||
# Local storage is stricter: response-scoped reasoning items (rs_*) cannot be replayed
|
||||
# back to the service unless that message is using service-side storage.
|
||||
# In that mode we omit reasoning items and rely on function call + tool output replay.
|
||||
has_function_call = any(c.type == "function_call" for c in message.contents)
|
||||
# Server-issued response item identities (function_call fc_*, reasoning rs_*, approval IDs,
|
||||
# local-shell-call IDs) must not be re-sent inline when the request carries
|
||||
# previous_response_id / conversation_id / conversation: the server already has them via
|
||||
# the prior response and rejects duplicates with "Duplicate item found with id ...".
|
||||
# function_result keeps its call_id and the server pairs it to the prior function_call via
|
||||
# that key. See microsoft/agent-framework#3295. The strip is gated on the request-level
|
||||
# flag, not a message-level one: HistoryProvider-attributed messages
|
||||
# (replays_local_storage) still need stripping when the request also carries a continuation
|
||||
# marker, since the server-stored items would otherwise duplicate the inline ones. Without
|
||||
# storage, standalone reasoning items are invalid per the API ("reasoning was provided
|
||||
# without its required following item"), so the reasoning branch always drops.
|
||||
for content in message.contents:
|
||||
match content.type:
|
||||
case "text_reasoning":
|
||||
if not uses_service_side_storage or not has_function_call:
|
||||
continue # reasoning not followed by a function_call is invalid in input
|
||||
reasoning = self._prepare_content_for_openai(
|
||||
message.role,
|
||||
content,
|
||||
replays_local_storage=replays_local_storage,
|
||||
)
|
||||
if reasoning:
|
||||
all_messages.append(reasoning)
|
||||
continue
|
||||
case "function_result":
|
||||
if request_uses_service_side_storage:
|
||||
props = content.additional_properties or {}
|
||||
# Local-shell variant serializes as `local_shell_call` carrying a server-issued id;
|
||||
# plain function_call_output pairs by call_id and is safe under storage.
|
||||
if (
|
||||
props.get(OPENAI_SHELL_OUTPUT_TYPE_KEY) == OPENAI_SHELL_OUTPUT_TYPE_LOCAL_SHELL_CALL
|
||||
and props.get(OPENAI_LOCAL_SHELL_CALL_ITEM_ID_KEY)
|
||||
):
|
||||
continue
|
||||
new_args: dict[str, Any] = {}
|
||||
new_args.update(
|
||||
self._prepare_content_for_openai(
|
||||
@@ -1443,6 +1445,8 @@ class RawOpenAIChatClient( # type: ignore[misc]
|
||||
if new_args:
|
||||
all_messages.append(new_args)
|
||||
case "function_call":
|
||||
if request_uses_service_side_storage:
|
||||
continue
|
||||
function_call = self._prepare_content_for_openai(
|
||||
message.role,
|
||||
content,
|
||||
@@ -1451,6 +1455,8 @@ class RawOpenAIChatClient( # type: ignore[misc]
|
||||
if function_call:
|
||||
all_messages.append(function_call)
|
||||
case "function_approval_response" | "function_approval_request":
|
||||
if request_uses_service_side_storage:
|
||||
continue
|
||||
prepared = self._prepare_content_for_openai(
|
||||
message.role,
|
||||
content,
|
||||
@@ -1463,6 +1469,12 @@ class RawOpenAIChatClient( # type: ignore[misc]
|
||||
# top-level mcp_call input item; the result side emits an
|
||||
# internal marker that `_prepare_messages_for_openai`
|
||||
# coalesces onto the matching call (or drops if unmatched).
|
||||
# The mcp_call item carries the model-emitted call_id as its
|
||||
# server-side `id`, so under continuation it would duplicate
|
||||
# the prior response's items (#3295). Drop the call here; the
|
||||
# orphan result is dropped by the coalesce step that follows.
|
||||
if request_uses_service_side_storage:
|
||||
continue
|
||||
prepared_mcp = self._prepare_content_for_openai(
|
||||
message.role,
|
||||
content,
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
# Copyright (c) Microsoft. All rights reserved.
|
||||
|
||||
import asyncio
|
||||
import base64
|
||||
import inspect
|
||||
import json
|
||||
@@ -121,15 +120,6 @@ async def create_vector_store(
|
||||
if result.last_error is not None:
|
||||
raise Exception(f"Vector store file processing failed with status: {result.last_error.message}")
|
||||
|
||||
# Wait for the vector store index to be fully searchable.
|
||||
# create_and_poll confirms file processing, but the search index is eventually consistent.
|
||||
for _ in range(10):
|
||||
vs = await client.client.vector_stores.retrieve(vector_store.id)
|
||||
if vs.file_counts.completed >= 1 and vs.file_counts.in_progress == 0:
|
||||
break
|
||||
await asyncio.sleep(1)
|
||||
await asyncio.sleep(2)
|
||||
|
||||
return file.id, Content.from_hosted_vector_store(vector_store_id=vector_store.id)
|
||||
|
||||
|
||||
@@ -343,76 +333,6 @@ async def test_get_response_with_all_parameters() -> None:
|
||||
assert run_options["input"][1]["content"][0]["text"] == "Test message"
|
||||
|
||||
|
||||
def test_openai_chat_options_declares_verbosity_field() -> None:
|
||||
"""OpenAIChatOptions declares verbosity as a typed Literal field."""
|
||||
from typing import get_args, get_type_hints
|
||||
|
||||
from agent_framework_openai import OpenAIChatOptions
|
||||
|
||||
annotations = get_type_hints(OpenAIChatOptions)
|
||||
assert "verbosity" in annotations
|
||||
assert {"low", "medium", "high"} <= set(get_args(annotations["verbosity"]))
|
||||
|
||||
|
||||
async def test_verbosity_option_translates_to_text_field() -> None:
|
||||
"""Top-level verbosity is translated to text.verbosity for the Responses API."""
|
||||
client = OpenAIChatClient(model="test-model", api_key="test-key")
|
||||
_, run_options, _ = await client._prepare_request(
|
||||
messages=[Message(role="user", contents=["Test message"])],
|
||||
options={"verbosity": "low"},
|
||||
)
|
||||
|
||||
assert "verbosity" not in run_options
|
||||
assert run_options["text"] == {"verbosity": "low"}
|
||||
|
||||
|
||||
async def test_verbosity_option_merges_with_response_format() -> None:
|
||||
"""Verbosity merges into text config alongside response_format-derived format."""
|
||||
client = OpenAIChatClient(model="test-model", api_key="test-key")
|
||||
_, run_options, _ = await client._prepare_request(
|
||||
messages=[Message(role="user", contents=["Test message"])],
|
||||
options={
|
||||
"verbosity": "high",
|
||||
"response_format": OutputStruct,
|
||||
},
|
||||
)
|
||||
|
||||
assert "verbosity" not in run_options
|
||||
assert run_options["text"]["verbosity"] == "high"
|
||||
assert run_options["text_format"] is OutputStruct
|
||||
|
||||
|
||||
async def test_verbosity_option_top_level_overrides_nested_text_verbosity() -> None:
|
||||
"""When both top-level and text['verbosity'] are set, the top-level value wins."""
|
||||
client = OpenAIChatClient(model="test-model", api_key="test-key")
|
||||
_, run_options, _ = await client._prepare_request(
|
||||
messages=[Message(role="user", contents=["Test message"])],
|
||||
options={
|
||||
"verbosity": "high",
|
||||
"text": {"verbosity": "low"},
|
||||
},
|
||||
)
|
||||
|
||||
assert "verbosity" not in run_options
|
||||
assert run_options["text"]["verbosity"] == "high"
|
||||
|
||||
|
||||
async def test_verbosity_option_merges_with_explicit_text_config() -> None:
|
||||
"""Verbosity merges into a user-provided text config without overwriting other keys."""
|
||||
client = OpenAIChatClient(model="test-model", api_key="test-key")
|
||||
_, run_options, _ = await client._prepare_request(
|
||||
messages=[Message(role="user", contents=["Test message"])],
|
||||
options={
|
||||
"verbosity": "medium",
|
||||
"text": {"format": {"type": "text"}},
|
||||
},
|
||||
)
|
||||
|
||||
assert "verbosity" not in run_options
|
||||
assert run_options["text"]["verbosity"] == "medium"
|
||||
assert run_options["text"]["format"] == {"type": "text"}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_web_search_tool_with_location() -> None:
|
||||
"""Test web search tool with location parameters."""
|
||||
@@ -518,7 +438,7 @@ async def test_chat_message_parsing_with_function_calls() -> None:
|
||||
Message(role="tool", contents=[function_result]),
|
||||
]
|
||||
|
||||
prepared_messages = client._prepare_messages_for_openai(messages)
|
||||
prepared_messages = client._prepare_messages_for_openai(messages, request_uses_service_side_storage=False)
|
||||
|
||||
assert prepared_messages == [
|
||||
{
|
||||
@@ -1834,7 +1754,7 @@ def test_prepare_message_for_openai_with_function_approval_response() -> None:
|
||||
|
||||
message = Message(role="user", contents=[approval_response])
|
||||
|
||||
result = client._prepare_message_for_openai(message)
|
||||
result = client._prepare_message_for_openai(message, request_uses_service_side_storage=False)
|
||||
|
||||
# FunctionApprovalResponseContent is added directly, not nested in args with role
|
||||
assert len(result) == 1
|
||||
@@ -1866,16 +1786,20 @@ def test_prepare_message_for_openai_includes_reasoning_with_function_call() -> N
|
||||
|
||||
message = Message(role="assistant", contents=[reasoning, function_call])
|
||||
|
||||
result = client._prepare_message_for_openai(message)
|
||||
# Storage-on path strips both server-issued reasoning (rs_*) and function_call items
|
||||
# because the server already has them via previous_response_id (#3295).
|
||||
storage_on_result = client._prepare_message_for_openai(message, request_uses_service_side_storage=True)
|
||||
storage_on_types = [item["type"] for item in storage_on_result]
|
||||
assert "reasoning" not in storage_on_types
|
||||
assert "function_call" not in storage_on_types
|
||||
|
||||
# Both reasoning and function_call should be present as top-level items
|
||||
types = [item["type"] for item in result]
|
||||
assert "reasoning" in types, "Reasoning items must be included for reasoning models"
|
||||
assert "function_call" in types
|
||||
|
||||
reasoning_item = next(item for item in result if item["type"] == "reasoning")
|
||||
assert reasoning_item["summary"][0]["text"] == "Let me analyze the request"
|
||||
assert reasoning_item["id"] == "rs_abc123", "Reasoning id must be preserved for the API"
|
||||
# Storage-off path keeps function_call inline so the server sees the call. Reasoning items
|
||||
# cannot be replayed inline against a server that has no record of the prior response, so
|
||||
# they remain dropped on this path as well.
|
||||
storage_off_result = client._prepare_message_for_openai(message, request_uses_service_side_storage=False)
|
||||
storage_off_types = [item["type"] for item in storage_off_result]
|
||||
assert "function_call" in storage_off_types
|
||||
assert "reasoning" not in storage_off_types
|
||||
|
||||
|
||||
def test_prepare_messages_for_openai_full_conversation_with_reasoning() -> None:
|
||||
@@ -1920,27 +1844,20 @@ def test_prepare_messages_for_openai_full_conversation_with_reasoning() -> None:
|
||||
),
|
||||
]
|
||||
|
||||
result = client._prepare_messages_for_openai(messages)
|
||||
# Storage-off path: function_call kept inline (server has no record of it),
|
||||
# function_call_output kept. Reasoning is still dropped because rs_* response-scoped IDs
|
||||
# cannot be replayed against a server that has no record of the originating response.
|
||||
result = client._prepare_messages_for_openai(messages, request_uses_service_side_storage=False)
|
||||
|
||||
types = [item.get("type") for item in result]
|
||||
assert "message" in types, "User/assistant messages should be present"
|
||||
assert "reasoning" in types, "Reasoning items must be present"
|
||||
assert "function_call" in types, "Function call items must be present"
|
||||
assert "function_call" in types, "Function call items must be present without storage"
|
||||
assert "function_call_output" in types, "Function call output must be present"
|
||||
|
||||
# Verify reasoning has id
|
||||
reasoning_items = [item for item in result if item.get("type") == "reasoning"]
|
||||
assert reasoning_items[0]["id"] == "rs_test123"
|
||||
|
||||
# Verify function_call has id
|
||||
fc_items = [item for item in result if item.get("type") == "function_call"]
|
||||
assert fc_items[0]["id"] == "fc_test456"
|
||||
|
||||
# Verify correct ordering: reasoning before function_call
|
||||
reasoning_idx = types.index("reasoning")
|
||||
fc_idx = types.index("function_call")
|
||||
assert reasoning_idx < fc_idx, "Reasoning must come before function_call"
|
||||
|
||||
|
||||
def test_prepare_message_for_openai_filters_error_content() -> None:
|
||||
"""Test that error content in messages is handled properly."""
|
||||
@@ -4082,7 +3999,13 @@ async def test_prepare_options_store_false_omits_reasoning_items_for_stateless_r
|
||||
assert any(item.get("type") == "function_call_output" for item in options["input"])
|
||||
|
||||
|
||||
async def test_prepare_options_with_conversation_id_keeps_reasoning_items() -> None:
|
||||
async def test_prepare_options_with_conversation_id_strips_server_issued_items() -> None:
|
||||
"""When the request continues via conversation_id / previous_response_id, server-issued
|
||||
response items (reasoning rs_*, function_call fc_*) must not be re-sent inline. The server
|
||||
already has them via the prior response and rejects duplicates with
|
||||
'Duplicate item found with id ...'. The function_result keeps its call_id so the server
|
||||
pairs result-to-call. See microsoft/agent-framework#3295. (Originally added in #5250 with
|
||||
the opposite expectation; field reports proved that path 400s on the wire.)"""
|
||||
client = OpenAIChatClient(model="test-model", api_key="test-key")
|
||||
messages = [
|
||||
Message(role="user", contents=[Content.from_text(text="search for hotels")]),
|
||||
@@ -4118,13 +4041,16 @@ async def test_prepare_options_with_conversation_id_keeps_reasoning_items() -> N
|
||||
ChatOptions(store=False, conversation_id="resp_prev123"), # type: ignore[arg-type]
|
||||
)
|
||||
|
||||
reasoning_items = [item for item in options["input"] if item.get("type") == "reasoning"]
|
||||
assert len(reasoning_items) == 1
|
||||
assert reasoning_items[0]["id"] == "rs_test123"
|
||||
types = [item.get("type") for item in options["input"]]
|
||||
assert "reasoning" not in types
|
||||
assert "function_call" not in types
|
||||
assert "function_call_output" in types
|
||||
output_item = next(item for item in options["input"] if item.get("type") == "function_call_output")
|
||||
assert output_item["call_id"] == "call_1"
|
||||
assert options["previous_response_id"] == "resp_prev123"
|
||||
|
||||
|
||||
async def test_prepare_options_with_conversation_id_omits_reasoning_items_for_attributed_replay() -> None:
|
||||
async def test_prepare_options_with_conversation_id_strips_server_items_for_mixed_history_and_live() -> None:
|
||||
client = OpenAIChatClient(model="test-model", api_key="test-key")
|
||||
messages = [
|
||||
Message(role="user", contents=[Content.from_text(text="search for hotels")]),
|
||||
@@ -4186,19 +4112,18 @@ async def test_prepare_options_with_conversation_id_omits_reasoning_items_for_at
|
||||
ChatOptions(store=False, conversation_id="resp_prev123"), # type: ignore[arg-type]
|
||||
)
|
||||
|
||||
reasoning_items = [item for item in options["input"] if item.get("type") == "reasoning"]
|
||||
assert [item["id"] for item in reasoning_items] == ["rs_live123"]
|
||||
assert any(
|
||||
item.get("type") == "function_call" and item.get("call_id") == "call_history" for item in options["input"]
|
||||
)
|
||||
assert any(item.get("type") == "function_call" and item.get("call_id") == "call_live" for item in options["input"])
|
||||
assert any(
|
||||
item.get("type") == "function_call_output" and item.get("call_id") == "call_history"
|
||||
for item in options["input"]
|
||||
)
|
||||
assert any(
|
||||
item.get("type") == "function_call_output" and item.get("call_id") == "call_live" for item in options["input"]
|
||||
)
|
||||
# Under continuation (request_uses_service_side_storage=True), the strip rule fires for
|
||||
# every server-issued item type regardless of message attribution: history-attributed items
|
||||
# would duplicate the prior response stored at resp_prev123, and live items would also
|
||||
# eventually duplicate items stored on the response this request produces. Function results
|
||||
# are kept; the server pairs them to prior function_calls via call_id (#3295).
|
||||
types = [item.get("type") for item in options["input"]]
|
||||
assert "reasoning" not in types
|
||||
assert "function_call" not in types
|
||||
output_call_ids = {
|
||||
item["call_id"] for item in options["input"] if item.get("type") == "function_call_output"
|
||||
}
|
||||
assert output_call_ids == {"call_history", "call_live"}
|
||||
assert options["previous_response_id"] == "resp_prev123"
|
||||
|
||||
|
||||
@@ -4465,6 +4390,10 @@ async def test_integration_web_search() -> None:
|
||||
assert response.text is not None
|
||||
|
||||
|
||||
@pytest.mark.skip(
|
||||
reason="Unreliable due to OpenAI vector store indexing potential "
|
||||
"race condition. See https://github.com/microsoft/agent-framework/issues/1669"
|
||||
)
|
||||
@pytest.mark.flaky
|
||||
@pytest.mark.integration
|
||||
@skip_if_openai_integration_tests_disabled
|
||||
@@ -4474,29 +4403,31 @@ async def test_integration_file_search() -> None:
|
||||
assert isinstance(openai_responses_client, SupportsChatGetResponse)
|
||||
|
||||
file_id, vector_store = await create_vector_store(openai_responses_client)
|
||||
try:
|
||||
# Use static method for file search tool
|
||||
file_search_tool = OpenAIChatClient.get_file_search_tool(vector_store_ids=[vector_store.vector_store_id])
|
||||
# Test that the client will use the file search tool
|
||||
response = await openai_responses_client.get_response(
|
||||
messages=[
|
||||
Message(
|
||||
role="user",
|
||||
contents=["What is the weather today? Do a file search to find the answer."],
|
||||
)
|
||||
],
|
||||
options={
|
||||
"tool_choice": "auto",
|
||||
"tools": [file_search_tool],
|
||||
},
|
||||
)
|
||||
# Use static method for file search tool
|
||||
file_search_tool = OpenAIChatClient.get_file_search_tool(vector_store_ids=[vector_store.vector_store_id])
|
||||
# Test that the client will use the file search tool
|
||||
response = await openai_responses_client.get_response(
|
||||
messages=[
|
||||
Message(
|
||||
role="user",
|
||||
contents=["What is the weather today? Do a file search to find the answer."],
|
||||
)
|
||||
],
|
||||
options={
|
||||
"tool_choice": "auto",
|
||||
"tools": [file_search_tool],
|
||||
},
|
||||
)
|
||||
|
||||
assert "sunny" in response.text.lower()
|
||||
assert "75" in response.text
|
||||
finally:
|
||||
await delete_vector_store(openai_responses_client, file_id, vector_store.vector_store_id)
|
||||
await delete_vector_store(openai_responses_client, file_id, vector_store.vector_store_id)
|
||||
assert "sunny" in response.text.lower()
|
||||
assert "75" in response.text
|
||||
|
||||
|
||||
@pytest.mark.skip(
|
||||
reason="Unreliable due to OpenAI vector store indexing "
|
||||
"potential race condition. See https://github.com/microsoft/agent-framework/issues/1669"
|
||||
)
|
||||
@pytest.mark.flaky
|
||||
@pytest.mark.integration
|
||||
@skip_if_openai_integration_tests_disabled
|
||||
@@ -4506,37 +4437,35 @@ async def test_integration_streaming_file_search() -> None:
|
||||
assert isinstance(openai_responses_client, SupportsChatGetResponse)
|
||||
|
||||
file_id, vector_store = await create_vector_store(openai_responses_client)
|
||||
try:
|
||||
# Use static method for file search tool
|
||||
file_search_tool = OpenAIChatClient.get_file_search_tool(vector_store_ids=[vector_store.vector_store_id])
|
||||
# Test that the client will use the file search tool
|
||||
response = openai_responses_client.get_response(
|
||||
messages=[
|
||||
Message(
|
||||
role="user",
|
||||
contents=["What is the weather today? Do a file search to find the answer."],
|
||||
)
|
||||
],
|
||||
stream=True,
|
||||
options={
|
||||
"tool_choice": "auto",
|
||||
"tools": [file_search_tool],
|
||||
},
|
||||
)
|
||||
# Use static method for file search tool
|
||||
file_search_tool = OpenAIChatClient.get_file_search_tool(vector_store_ids=[vector_store.vector_store_id])
|
||||
# Test that the client will use the web search tool
|
||||
response = openai_responses_client.get_streaming_response(
|
||||
messages=[
|
||||
Message(
|
||||
role="user",
|
||||
contents=["What is the weather today? Do a file search to find the answer."],
|
||||
)
|
||||
],
|
||||
options={
|
||||
"tool_choice": "auto",
|
||||
"tools": [file_search_tool],
|
||||
},
|
||||
)
|
||||
|
||||
assert response is not None
|
||||
full_message: str = ""
|
||||
async for chunk in response:
|
||||
assert chunk is not None
|
||||
assert isinstance(chunk, ChatResponseUpdate)
|
||||
for content in chunk.contents:
|
||||
if content.type == "text" and content.text:
|
||||
full_message += content.text
|
||||
assert response is not None
|
||||
full_message: str = ""
|
||||
async for chunk in response:
|
||||
assert chunk is not None
|
||||
assert isinstance(chunk, ChatResponseUpdate)
|
||||
for content in chunk.contents:
|
||||
if content.type == "text" and content.text:
|
||||
full_message += content.text
|
||||
|
||||
assert "sunny" in full_message.lower()
|
||||
assert "75" in full_message
|
||||
finally:
|
||||
await delete_vector_store(openai_responses_client, file_id, vector_store.vector_store_id)
|
||||
await delete_vector_store(openai_responses_client, file_id, vector_store.vector_store_id)
|
||||
|
||||
assert "sunny" in full_message.lower()
|
||||
assert "75" in full_message
|
||||
|
||||
|
||||
@pytest.mark.flaky
|
||||
@@ -5059,7 +4988,10 @@ async def test_prepare_messages_for_openai_does_not_replay_fc_id_when_loaded_fro
|
||||
|
||||
next_turn_input = Message(role="user", contents=[Content.from_text(text="Book the cheapest one")])
|
||||
|
||||
live_result = client._prepare_messages_for_openai([*session.state[provider.source_id]["messages"], next_turn_input])
|
||||
live_result = client._prepare_messages_for_openai(
|
||||
[*session.state[provider.source_id]["messages"], next_turn_input],
|
||||
request_uses_service_side_storage=False,
|
||||
)
|
||||
live_function_call = next(item for item in live_result if item.get("type") == "function_call")
|
||||
assert live_function_call["id"] == "fc_provider123"
|
||||
|
||||
@@ -5072,7 +5004,8 @@ async def test_prepare_messages_for_openai_does_not_replay_fc_id_when_loaded_fro
|
||||
) # type: ignore[arg-type]
|
||||
|
||||
loaded_result = client._prepare_messages_for_openai(
|
||||
context.get_messages(sources={provider.source_id}, include_input=True)
|
||||
context.get_messages(sources={provider.source_id}, include_input=True),
|
||||
request_uses_service_side_storage=False,
|
||||
)
|
||||
loaded_function_call = next(item for item in loaded_result if item.get("type") == "function_call")
|
||||
assert loaded_function_call["id"] == "fc_call_1"
|
||||
@@ -5091,7 +5024,8 @@ async def test_prepare_messages_for_openai_does_not_replay_fc_id_when_loaded_fro
|
||||
) # type: ignore[arg-type]
|
||||
|
||||
restored_result = client._prepare_messages_for_openai(
|
||||
restored_context.get_messages(sources={provider.source_id}, include_input=True)
|
||||
restored_context.get_messages(sources={provider.source_id}, include_input=True),
|
||||
request_uses_service_side_storage=False,
|
||||
)
|
||||
restored_function_call = next(item for item in restored_result if item.get("type") == "function_call")
|
||||
assert restored_function_call["id"] == "fc_call_1"
|
||||
@@ -5125,7 +5059,9 @@ def test_prepare_messages_for_openai_keeps_live_fc_id_separate_from_replayed_his
|
||||
],
|
||||
)
|
||||
|
||||
result = client._prepare_messages_for_openai([history_message, live_message])
|
||||
result = client._prepare_messages_for_openai(
|
||||
[history_message, live_message], request_uses_service_side_storage=False
|
||||
)
|
||||
|
||||
function_calls = [item for item in result if item.get("type") == "function_call"]
|
||||
assert [item["id"] for item in function_calls] == ["fc_call_1", "fc_live123"]
|
||||
@@ -5163,7 +5099,7 @@ def test_prepare_messages_for_openai_filters_empty_fc_id() -> None:
|
||||
),
|
||||
]
|
||||
|
||||
result = client._prepare_messages_for_openai(messages)
|
||||
result = client._prepare_messages_for_openai(messages, request_uses_service_side_storage=False)
|
||||
|
||||
# Find the function_call items in the result
|
||||
fc_items = [item for item in result if item.get("type") == "function_call"]
|
||||
@@ -5198,7 +5134,7 @@ def test_prepare_messages_for_openai_filters_none_fc_id() -> None:
|
||||
),
|
||||
]
|
||||
|
||||
result = client._prepare_messages_for_openai(messages)
|
||||
result = client._prepare_messages_for_openai(messages, request_uses_service_side_storage=False)
|
||||
|
||||
# Find the function_call item
|
||||
fc_items = [item for item in result if item.get("type") == "function_call"]
|
||||
@@ -5233,7 +5169,7 @@ def test_prepare_messages_for_openai_serializes_mcp_server_tool_call_as_mcp_call
|
||||
),
|
||||
]
|
||||
|
||||
result = client._prepare_messages_for_openai(messages)
|
||||
result = client._prepare_messages_for_openai(messages, request_uses_service_side_storage=False)
|
||||
|
||||
mcp_items = [item for item in result if isinstance(item, dict) and item.get("type") == "mcp_call"]
|
||||
assert len(mcp_items) == 1, f"expected exactly one mcp_call item; got result={result}"
|
||||
@@ -5276,7 +5212,7 @@ def test_prepare_messages_for_openai_coalesces_mcp_call_and_result_into_single_i
|
||||
),
|
||||
]
|
||||
|
||||
result = client._prepare_messages_for_openai(messages)
|
||||
result = client._prepare_messages_for_openai(messages, request_uses_service_side_storage=False)
|
||||
|
||||
mcp_items = [item for item in result if isinstance(item, dict) and item.get("type") == "mcp_call"]
|
||||
assert len(mcp_items) == 1, f"expected one coalesced mcp_call item carrying both arguments and output; got {result}"
|
||||
@@ -5310,7 +5246,7 @@ def test_prepare_messages_for_openai_drops_orphan_mcp_server_tool_result() -> No
|
||||
),
|
||||
]
|
||||
|
||||
result = client._prepare_messages_for_openai(messages)
|
||||
result = client._prepare_messages_for_openai(messages, request_uses_service_side_storage=False)
|
||||
|
||||
fco_items = [item for item in result if isinstance(item, dict) and item.get("type") == "function_call_output"]
|
||||
assert fco_items == [], f"orphan mcp_server_tool_result must not serialize as function_call_output; got {fco_items}"
|
||||
@@ -5342,4 +5278,170 @@ def test_stringify_mcp_output_falls_back_to_json_for_non_text_dict_entries() ->
|
||||
# endregion
|
||||
|
||||
|
||||
# region: strip server-issued item IDs under storage (issue #3295)
|
||||
|
||||
|
||||
def _strip_rule_messages() -> list[Message]:
|
||||
return [
|
||||
Message(role="user", contents=[Content.from_text(text="search hotels in Paris")]),
|
||||
Message(
|
||||
role="assistant",
|
||||
contents=[
|
||||
Content.from_function_call(
|
||||
call_id="call_1",
|
||||
name="search_hotels",
|
||||
arguments='{"city": "Paris"}',
|
||||
additional_properties={"fc_id": "fc_server_issued"},
|
||||
),
|
||||
],
|
||||
),
|
||||
Message(
|
||||
role="tool",
|
||||
contents=[Content.from_function_result(call_id="call_1", result="Found 3 hotels in Paris")],
|
||||
),
|
||||
]
|
||||
|
||||
|
||||
def test_prepare_messages_strips_function_call_under_storage() -> None:
|
||||
"""Regression for #3295: when previous_response_id / conversation_id is in flight, the chat
|
||||
client must not re-send server-issued function_call items inline. The server already has them
|
||||
via the prior response and rejects duplicates with 'Duplicate item found with id fc_...'.
|
||||
The function_result keeps its call_id so the server can pair result-to-call."""
|
||||
client = OpenAIChatClient(model="test-model", api_key="test-key")
|
||||
|
||||
result = client._prepare_messages_for_openai(_strip_rule_messages(), request_uses_service_side_storage=True)
|
||||
|
||||
types = [item.get("type") for item in result]
|
||||
assert "function_call" not in types
|
||||
assert "function_call_output" in types
|
||||
output_item = next(item for item in result if item.get("type") == "function_call_output")
|
||||
assert output_item["call_id"] == "call_1"
|
||||
|
||||
|
||||
def test_prepare_messages_keeps_function_call_without_storage() -> None:
|
||||
"""Without storage there is no previous_response_id, so inline function_call items are the
|
||||
only source of truth for the server. Behavior is byte-identical to pre-#3295."""
|
||||
client = OpenAIChatClient(model="test-model", api_key="test-key")
|
||||
|
||||
result = client._prepare_messages_for_openai(_strip_rule_messages(), request_uses_service_side_storage=False)
|
||||
|
||||
types = [item.get("type") for item in result]
|
||||
assert "function_call" in types
|
||||
assert "function_call_output" in types
|
||||
fc_item = next(item for item in result if item.get("type") == "function_call")
|
||||
assert fc_item["call_id"] == "call_1"
|
||||
assert fc_item["id"] == "fc_server_issued"
|
||||
output_item = next(item for item in result if item.get("type") == "function_call_output")
|
||||
assert output_item["call_id"] == "call_1"
|
||||
|
||||
|
||||
def test_prepare_messages_strips_approval_items_under_storage() -> None:
|
||||
"""Approval request/response items also carry server-issued IDs and must be stripped under
|
||||
storage. Without storage they are kept (#3295)."""
|
||||
client = OpenAIChatClient(model="test-model", api_key="test-key")
|
||||
|
||||
function_call = Content.from_function_call(
|
||||
call_id="mcp_1",
|
||||
name="sensitive_action",
|
||||
arguments='{"action": "delete"}',
|
||||
)
|
||||
approval_request = Content.from_function_approval_request(
|
||||
id="approval_req_1",
|
||||
function_call=function_call,
|
||||
)
|
||||
approval_response = Content.from_function_approval_response(
|
||||
approved=True,
|
||||
id="approval_req_1",
|
||||
function_call=function_call,
|
||||
)
|
||||
messages = [
|
||||
Message(role="assistant", contents=[approval_request]),
|
||||
Message(role="user", contents=[approval_response]),
|
||||
]
|
||||
|
||||
storage_on = client._prepare_messages_for_openai(messages, request_uses_service_side_storage=True)
|
||||
storage_on_types = [item.get("type") for item in storage_on]
|
||||
assert "mcp_approval_request" not in storage_on_types
|
||||
assert "mcp_approval_response" not in storage_on_types
|
||||
|
||||
storage_off = client._prepare_messages_for_openai(messages, request_uses_service_side_storage=False)
|
||||
storage_off_types = [item.get("type") for item in storage_off]
|
||||
assert "mcp_approval_request" in storage_off_types
|
||||
assert "mcp_approval_response" in storage_off_types
|
||||
|
||||
|
||||
def test_prepare_messages_strips_local_shell_call_under_storage() -> None:
|
||||
"""Local-shell-call function_results carry a server-issued local_shell_call_item_id and must
|
||||
be stripped under storage. Plain function_results (no shell ID) are kept either way (#3295)."""
|
||||
from agent_framework_openai._chat_client import (
|
||||
OPENAI_LOCAL_SHELL_CALL_ITEM_ID_KEY,
|
||||
OPENAI_SHELL_OUTPUT_TYPE_KEY,
|
||||
OPENAI_SHELL_OUTPUT_TYPE_LOCAL_SHELL_CALL,
|
||||
)
|
||||
|
||||
client = OpenAIChatClient(model="test-model", api_key="test-key")
|
||||
shell_result = Content.from_function_result(
|
||||
call_id="shell_1",
|
||||
result="ok",
|
||||
additional_properties={
|
||||
OPENAI_SHELL_OUTPUT_TYPE_KEY: OPENAI_SHELL_OUTPUT_TYPE_LOCAL_SHELL_CALL,
|
||||
OPENAI_LOCAL_SHELL_CALL_ITEM_ID_KEY: "lsh_server_issued",
|
||||
},
|
||||
)
|
||||
plain_result = Content.from_function_result(call_id="plain_1", result="plain")
|
||||
message = Message(role="tool", contents=[shell_result, plain_result])
|
||||
|
||||
storage_on = client._prepare_message_for_openai(message, request_uses_service_side_storage=True)
|
||||
types_on = [item.get("type") for item in storage_on]
|
||||
assert OPENAI_SHELL_OUTPUT_TYPE_LOCAL_SHELL_CALL not in types_on
|
||||
assert "function_call_output" in types_on
|
||||
|
||||
storage_off = client._prepare_message_for_openai(message, request_uses_service_side_storage=False)
|
||||
types_off = [item.get("type") for item in storage_off]
|
||||
assert OPENAI_SHELL_OUTPUT_TYPE_LOCAL_SHELL_CALL in types_off
|
||||
assert "function_call_output" in types_off
|
||||
|
||||
|
||||
def test_prepare_messages_strips_mcp_items_under_storage() -> None:
|
||||
"""Hosted-MCP tool call items carry server-issued IDs (the call_id surfaces as `id` on the
|
||||
wire mcp_call item), so they must be stripped under storage. The orphan mcp_server_tool_result
|
||||
is then dropped by the existing coalesce logic (#5581). Without storage, the call/result pair
|
||||
coalesces normally into a single mcp_call wire item (#3295)."""
|
||||
client = OpenAIChatClient(model="test-model", api_key="test-key")
|
||||
|
||||
messages = [
|
||||
Message(
|
||||
role="assistant",
|
||||
contents=[
|
||||
Content.from_mcp_server_tool_call(
|
||||
call_id="mcp_abc123",
|
||||
tool_name="search",
|
||||
server_name="api_specs",
|
||||
arguments='{"q": "cats"}',
|
||||
)
|
||||
],
|
||||
),
|
||||
Message(
|
||||
role="tool",
|
||||
contents=[
|
||||
Content.from_mcp_server_tool_result(
|
||||
call_id="mcp_abc123",
|
||||
output=[Content.from_text(text="found 10 cats")],
|
||||
)
|
||||
],
|
||||
),
|
||||
]
|
||||
|
||||
storage_on = client._prepare_messages_for_openai(messages, request_uses_service_side_storage=True)
|
||||
storage_on_types = [item.get("type") for item in storage_on]
|
||||
assert "mcp_call" not in storage_on_types
|
||||
|
||||
storage_off = client._prepare_messages_for_openai(messages, request_uses_service_side_storage=False)
|
||||
storage_off_types = [item.get("type") for item in storage_off]
|
||||
assert "mcp_call" in storage_off_types
|
||||
|
||||
|
||||
# endregion
|
||||
|
||||
|
||||
# endregion
|
||||
|
||||
@@ -11,7 +11,7 @@ import os
|
||||
from textwrap import dedent
|
||||
from typing import Any
|
||||
|
||||
from agent_framework import Agent, InlineSkill, InlineSkillResource, SkillsProvider
|
||||
from agent_framework import Agent, InlineSkill, InlineSkillResource, SkillFrontmatter, SkillsProvider
|
||||
from agent_framework.foundry import FoundryChatClient
|
||||
from azure.identity import AzureCliCredential
|
||||
from dotenv import load_dotenv
|
||||
@@ -47,8 +47,9 @@ load_dotenv()
|
||||
# 1. Static Resources — inline content passed at construction time
|
||||
# ---------------------------------------------------------------------------
|
||||
unit_converter_skill = InlineSkill(
|
||||
name="unit-converter",
|
||||
description="Convert between common units using a conversion factor",
|
||||
frontmatter=SkillFrontmatter(
|
||||
name="unit-converter", description="Convert between common units using a conversion factor"
|
||||
),
|
||||
instructions=dedent("""\
|
||||
Use this skill when the user asks to convert between units.
|
||||
|
||||
|
||||
@@ -1,6 +1,12 @@
|
||||
---
|
||||
name: unit-converter
|
||||
description: Convert between common units using a multiplication factor. Use when asked to convert miles, kilometers, pounds, or kilograms.
|
||||
license: MIT
|
||||
compatibility: Works with any model that supports tool use.
|
||||
allowed-tools: convert
|
||||
metadata:
|
||||
author: agent-framework-samples
|
||||
version: "1.0"
|
||||
---
|
||||
|
||||
## Usage
|
||||
|
||||
@@ -21,6 +21,7 @@ from agent_framework import (
|
||||
FileSkillsSource,
|
||||
InlineSkill,
|
||||
InMemorySkillsSource,
|
||||
SkillFrontmatter,
|
||||
SkillsProvider,
|
||||
)
|
||||
from agent_framework.foundry import FoundryChatClient
|
||||
@@ -73,8 +74,9 @@ load_dotenv()
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
volume_converter_skill = InlineSkill(
|
||||
name="volume-converter",
|
||||
description="Convert between gallons and liters using a conversion factor",
|
||||
frontmatter=SkillFrontmatter(
|
||||
name="volume-converter", description="Convert between gallons and liters using a conversion factor"
|
||||
),
|
||||
instructions=dedent("""\
|
||||
Use this skill when the user asks to convert between gallons and liters.
|
||||
|
||||
@@ -118,6 +120,7 @@ def convert_volume(value: float, factor: float) -> str:
|
||||
# 2. Define a class-based skill for temperature conversion
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TemperatureConverterSkill(ClassSkill):
|
||||
"""A temperature-converter skill defined as a Python class.
|
||||
|
||||
@@ -127,8 +130,10 @@ class TemperatureConverterSkill(ClassSkill):
|
||||
|
||||
def __init__(self) -> None:
|
||||
super().__init__(
|
||||
name="temperature-converter",
|
||||
description="Convert between temperature scales (Fahrenheit, Celsius, Kelvin).",
|
||||
frontmatter=SkillFrontmatter(
|
||||
name="temperature-converter",
|
||||
description="Convert between temperature scales (Fahrenheit, Celsius, Kelvin).",
|
||||
)
|
||||
)
|
||||
|
||||
@property
|
||||
@@ -178,6 +183,7 @@ class TemperatureConverterSkill(ClassSkill):
|
||||
# 3. Wire everything together and run the agent
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
async def main() -> None:
|
||||
"""Run the combined skills demo."""
|
||||
endpoint = os.environ["FOUNDRY_PROJECT_ENDPOINT"]
|
||||
|
||||
@@ -1,6 +1,12 @@
|
||||
---
|
||||
name: unit-converter
|
||||
description: Convert between common units using a multiplication factor. Use when asked to convert miles, kilometers, pounds, or kilograms.
|
||||
license: MIT
|
||||
compatibility: Works with any model that supports tool use.
|
||||
allowed-tools: convert
|
||||
metadata:
|
||||
author: agent-framework-samples
|
||||
version: "1.0"
|
||||
---
|
||||
|
||||
## Usage
|
||||
|
||||
@@ -9,7 +9,7 @@ import os
|
||||
# warnings.filterwarnings("ignore", message=r"\[SKILLS\].*", category=FutureWarning)
|
||||
from textwrap import dedent
|
||||
|
||||
from agent_framework import Agent, InlineSkill, SkillsProvider
|
||||
from agent_framework import Agent, InlineSkill, SkillFrontmatter, SkillsProvider
|
||||
from agent_framework.foundry import FoundryChatClient
|
||||
from azure.identity import AzureCliCredential
|
||||
from dotenv import load_dotenv
|
||||
@@ -43,8 +43,9 @@ load_dotenv()
|
||||
|
||||
# Define a code skill with a script that performs a sensitive operation
|
||||
deployment_skill = InlineSkill(
|
||||
name="deployment",
|
||||
description="Tools for deploying application versions to production",
|
||||
frontmatter=SkillFrontmatter(
|
||||
name="deployment", description="Tools for deploying application versions to production"
|
||||
),
|
||||
instructions=dedent("""\
|
||||
Use this skill when the user asks to deploy an application.
|
||||
|
||||
|
||||
@@ -75,7 +75,7 @@ async def main() -> None:
|
||||
FilteringSkillsSource(
|
||||
FileSkillsSource(str(skills_dir), script_runner=subprocess_script_runner),
|
||||
# Only keep the volume-converter skill
|
||||
predicate=lambda s: s.name != "length-converter",
|
||||
predicate=lambda s: s.frontmatter.name != "length-converter",
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
@@ -1,6 +1,12 @@
|
||||
---
|
||||
name: length-converter
|
||||
description: Convert between common length units (miles, km, feet, meters) using a multiplication factor.
|
||||
license: MIT
|
||||
compatibility: Works with any model that supports tool use.
|
||||
allowed-tools: convert
|
||||
metadata:
|
||||
author: agent-framework-samples
|
||||
version: "1.0"
|
||||
---
|
||||
|
||||
## Usage
|
||||
|
||||
@@ -1,6 +1,12 @@
|
||||
---
|
||||
name: volume-converter
|
||||
description: Convert between gallons and liters using a conversion factor.
|
||||
license: MIT
|
||||
compatibility: Works with any model that supports tool use.
|
||||
allowed-tools: convert
|
||||
metadata:
|
||||
author: agent-framework-samples
|
||||
version: "1.0"
|
||||
---
|
||||
|
||||
## Usage
|
||||
|
||||
Reference in New Issue
Block a user