From 08541ee5a9912b2d9b6b95b63ae3ea4ffea05cc1 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh <68852919+SergeyMenshykh@users.noreply.github.com> Date: Mon, 25 May 2026 18:16:03 +0100 Subject: [PATCH] .NET: [Breaking] Refactor AgentSkill API to async resource and script lookup (#6030) * .NET: Refactor AgentSkill API to async resource and script lookup Replace property-based AgentSkill.Content, Resources, and Scripts with async-by-name lookup methods plus boolean availability flags: - Content (string getter) -> GetContentAsync(CancellationToken) - Resources (full list) -> HasResources + GetResourceAsync(name, ct) - Scripts (full list) -> HasScripts + GetScriptAsync(name, ct) This makes the API friendlier for sources like MCP where enumerating all resources up front is expensive or impossible, and allows skill implementations to fetch content lazily. Subclass changes: - AgentFileSkill and AgentInlineSkill implement the new async API while preserving content caching. - AgentClassSkill keeps virtual Resources/Scripts properties for reflection-based discovery and seals the new HasResources/HasScripts/ GetResourceAsync/GetScriptAsync overrides. Its previously non-thread-safe lazy initialization is replaced with Lazy (default thread-safety) wired up in a new protected constructor, so concurrent first-access from multiple threads is safe. - AgentSkillsProvider calls the new async API and exposes ead_skill_resource / load_skill / un_skill_script tools that await the per-name lookups. Includes baseline CompatibilitySuppressions.xml entries for the removed property getters. Tests: - Direct coverage for HasResources, HasScripts, GetResourceAsync, and GetScriptAsync on all three skill implementations (positive, missing-name, and no-resources/no-scripts cases). - Thread-safety regression test for AgentClassSkill that exercises concurrent first-access to Resources, Scripts, and GetContentAsync from many tasks and asserts all observers see the same cached instance. - Provider-level coverage for the ead_skill_resource tool (invocation + error paths) and for the previously untested error paths of load_skill and un_skill_script (empty names, skill/resource/script not found). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR review comments - Move GetScriptAsync inside try/catch in RunSkillScriptAsync for error-handling parity - Remove dead _reflectedResources branch from AgentSkillTestExtensions - Fix XML docs to reference virtual Resources/Scripts properties (not sealed methods) - Add Async suffix to async test methods per naming convention - Make no-await tests synchronous to eliminate CS1998 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix formatting: add UTF-8 BOM and remove unused using Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix XML cref: Resources/Scripts are on AgentClassSkill Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove HasResources and HasScripts properties from AgentSkill Drop the virtual HasResources and HasScripts properties from AgentSkill and all concrete subclasses (AgentFileSkill, AgentInlineSkill, AgentClassSkill). AgentSkillsProvider now always includes all three tools (load_skill, read_skill_resource, run_skill_script) and both instruction blocks, since the tools already handle missing resources/scripts gracefully. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add blank line for readability in file-based skills sample Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix HostedAgentSkillsPatternTests for always-included tools Update assertions to expect read_skill_resource and run_skill_script tools are always present, matching the new behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Agent_Step01_FileBasedSkills/Program.cs | 1 + .../Agent_Step03_ClassBasedSkills/Program.cs | 2 +- .../CompatibilitySuppressions.xml | 140 +++++++++ .../Microsoft.Agents.AI/Skills/AgentSkill.cs | 40 ++- .../Skills/AgentSkillsProvider.cs | 77 ++--- .../Skills/File/AgentFileSkill.cs | 20 +- .../Skills/Programmatic/AgentClassSkill.cs | 102 +++--- .../Skills/Programmatic/AgentInlineSkill.cs | 24 +- .../AgentSkillResourceAttribute.cs | 2 +- .../Programmatic/AgentSkillScriptAttribute.cs | 2 +- .../AgentSkills/AgentClassSkillTests.cs | 200 +++++++++--- .../AgentSkills/AgentFileSkillScriptTests.cs | 18 +- .../AgentFileSkillsSourceScriptTests.cs | 50 +-- .../AgentSkills/AgentInlineSkillTests.cs | 168 +++++++--- .../AgentSkills/AgentSkillTestExtensions.cs | 43 +++ .../AgentSkills/AgentSkillsProviderTests.cs | 292 +++++++++++++++++- .../AgentSkills/FileAgentSkillLoaderTests.cs | 86 +++--- .../HostedAgentSkillsPatternTests.cs | 6 +- .../AgentSkills/TestSkillTypes.cs | 8 +- 19 files changed, 979 insertions(+), 302 deletions(-) create mode 100644 dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillTestExtensions.cs diff --git a/dotnet/samples/02-agents/AgentSkills/Agent_Step01_FileBasedSkills/Program.cs b/dotnet/samples/02-agents/AgentSkills/Agent_Step01_FileBasedSkills/Program.cs index f6b9b58b79..c9dc86e3b4 100644 --- a/dotnet/samples/02-agents/AgentSkills/Agent_Step01_FileBasedSkills/Program.cs +++ b/dotnet/samples/02-agents/AgentSkills/Agent_Step01_FileBasedSkills/Program.cs @@ -24,6 +24,7 @@ string deploymentName = Environment.GetEnvironmentVariable("AZURE_OPENAI_DEPLOYM var skillsProvider = new AgentSkillsProvider( Path.Combine(AppContext.BaseDirectory, "skills"), SubprocessScriptRunner.RunAsync); + // --- Agent Setup --- AIAgent agent = new AzureOpenAIClient(new Uri(endpoint), new DefaultAzureCredential()) .GetResponsesClient() diff --git a/dotnet/samples/02-agents/AgentSkills/Agent_Step03_ClassBasedSkills/Program.cs b/dotnet/samples/02-agents/AgentSkills/Agent_Step03_ClassBasedSkills/Program.cs index 7f5e356a60..aa70c4461e 100644 --- a/dotnet/samples/02-agents/AgentSkills/Agent_Step03_ClassBasedSkills/Program.cs +++ b/dotnet/samples/02-agents/AgentSkills/Agent_Step03_ClassBasedSkills/Program.cs @@ -51,7 +51,7 @@ Console.WriteLine($"Agent: {response.Text}"); /// Properties annotated with are automatically /// discovered as skill resources, and methods annotated with /// are automatically discovered as skill scripts. Alternatively, -/// and can be overridden. +/// and can be overridden. /// internal sealed class UnitConverterSkill : AgentClassSkill { diff --git a/dotnet/src/Microsoft.Agents.AI/CompatibilitySuppressions.xml b/dotnet/src/Microsoft.Agents.AI/CompatibilitySuppressions.xml index 6a2c790f22..a7f87560bf 100644 --- a/dotnet/src/Microsoft.Agents.AI/CompatibilitySuppressions.xml +++ b/dotnet/src/Microsoft.Agents.AI/CompatibilitySuppressions.xml @@ -43,6 +43,27 @@ lib/net10.0/Microsoft.Agents.AI.dll true + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Content + lib/net10.0/Microsoft.Agents.AI.dll + lib/net10.0/Microsoft.Agents.AI.dll + true + + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Resources + lib/net10.0/Microsoft.Agents.AI.dll + lib/net10.0/Microsoft.Agents.AI.dll + true + + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Scripts + lib/net10.0/Microsoft.Agents.AI.dll + lib/net10.0/Microsoft.Agents.AI.dll + true + CP0002 M:Microsoft.Agents.AI.AgentSkillScript.RunAsync(Microsoft.Agents.AI.AgentSkill,Microsoft.Extensions.AI.AIFunctionArguments,System.Threading.CancellationToken) @@ -106,6 +127,27 @@ lib/net472/Microsoft.Agents.AI.dll true + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Content + lib/net472/Microsoft.Agents.AI.dll + lib/net472/Microsoft.Agents.AI.dll + true + + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Resources + lib/net472/Microsoft.Agents.AI.dll + lib/net472/Microsoft.Agents.AI.dll + true + + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Scripts + lib/net472/Microsoft.Agents.AI.dll + lib/net472/Microsoft.Agents.AI.dll + true + CP0002 M:Microsoft.Agents.AI.AgentSkillScript.RunAsync(Microsoft.Agents.AI.AgentSkill,Microsoft.Extensions.AI.AIFunctionArguments,System.Threading.CancellationToken) @@ -169,6 +211,27 @@ lib/net8.0/Microsoft.Agents.AI.dll true + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Content + lib/net8.0/Microsoft.Agents.AI.dll + lib/net8.0/Microsoft.Agents.AI.dll + true + + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Resources + lib/net8.0/Microsoft.Agents.AI.dll + lib/net8.0/Microsoft.Agents.AI.dll + true + + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Scripts + lib/net8.0/Microsoft.Agents.AI.dll + lib/net8.0/Microsoft.Agents.AI.dll + true + CP0002 M:Microsoft.Agents.AI.AgentSkillScript.RunAsync(Microsoft.Agents.AI.AgentSkill,Microsoft.Extensions.AI.AIFunctionArguments,System.Threading.CancellationToken) @@ -232,6 +295,27 @@ lib/net9.0/Microsoft.Agents.AI.dll true + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Content + lib/net9.0/Microsoft.Agents.AI.dll + lib/net9.0/Microsoft.Agents.AI.dll + true + + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Resources + lib/net9.0/Microsoft.Agents.AI.dll + lib/net9.0/Microsoft.Agents.AI.dll + true + + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Scripts + lib/net9.0/Microsoft.Agents.AI.dll + lib/net9.0/Microsoft.Agents.AI.dll + true + CP0002 M:Microsoft.Agents.AI.AgentSkillScript.RunAsync(Microsoft.Agents.AI.AgentSkill,Microsoft.Extensions.AI.AIFunctionArguments,System.Threading.CancellationToken) @@ -295,6 +379,27 @@ lib/netstandard2.0/Microsoft.Agents.AI.dll true + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Content + lib/netstandard2.0/Microsoft.Agents.AI.dll + lib/netstandard2.0/Microsoft.Agents.AI.dll + true + + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Resources + lib/netstandard2.0/Microsoft.Agents.AI.dll + lib/netstandard2.0/Microsoft.Agents.AI.dll + true + + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Scripts + lib/netstandard2.0/Microsoft.Agents.AI.dll + lib/netstandard2.0/Microsoft.Agents.AI.dll + true + CP0002 M:Microsoft.Agents.AI.AgentSkillScript.RunAsync(Microsoft.Agents.AI.AgentSkill,Microsoft.Extensions.AI.AIFunctionArguments,System.Threading.CancellationToken) @@ -316,6 +421,13 @@ lib/netstandard2.0/Microsoft.Agents.AI.dll true + + CP0005 + M:Microsoft.Agents.AI.AgentSkill.GetContentAsync(System.Threading.CancellationToken) + lib/net10.0/Microsoft.Agents.AI.dll + lib/net10.0/Microsoft.Agents.AI.dll + true + CP0005 M:Microsoft.Agents.AI.AgentSkillScript.RunAsync(Microsoft.Agents.AI.AgentSkill,System.Nullable{System.Text.Json.JsonElement},System.IServiceProvider,System.Threading.CancellationToken) @@ -323,6 +435,13 @@ lib/net10.0/Microsoft.Agents.AI.dll true + + CP0005 + M:Microsoft.Agents.AI.AgentSkill.GetContentAsync(System.Threading.CancellationToken) + lib/net472/Microsoft.Agents.AI.dll + lib/net472/Microsoft.Agents.AI.dll + true + CP0005 M:Microsoft.Agents.AI.AgentSkillScript.RunAsync(Microsoft.Agents.AI.AgentSkill,System.Nullable{System.Text.Json.JsonElement},System.IServiceProvider,System.Threading.CancellationToken) @@ -330,6 +449,13 @@ lib/net472/Microsoft.Agents.AI.dll true + + CP0005 + M:Microsoft.Agents.AI.AgentSkill.GetContentAsync(System.Threading.CancellationToken) + lib/net8.0/Microsoft.Agents.AI.dll + lib/net8.0/Microsoft.Agents.AI.dll + true + CP0005 M:Microsoft.Agents.AI.AgentSkillScript.RunAsync(Microsoft.Agents.AI.AgentSkill,System.Nullable{System.Text.Json.JsonElement},System.IServiceProvider,System.Threading.CancellationToken) @@ -337,6 +463,13 @@ lib/net8.0/Microsoft.Agents.AI.dll true + + CP0005 + M:Microsoft.Agents.AI.AgentSkill.GetContentAsync(System.Threading.CancellationToken) + lib/net9.0/Microsoft.Agents.AI.dll + lib/net9.0/Microsoft.Agents.AI.dll + true + CP0005 M:Microsoft.Agents.AI.AgentSkillScript.RunAsync(Microsoft.Agents.AI.AgentSkill,System.Nullable{System.Text.Json.JsonElement},System.IServiceProvider,System.Threading.CancellationToken) @@ -344,6 +477,13 @@ lib/net9.0/Microsoft.Agents.AI.dll true + + CP0005 + M:Microsoft.Agents.AI.AgentSkill.GetContentAsync(System.Threading.CancellationToken) + lib/netstandard2.0/Microsoft.Agents.AI.dll + lib/netstandard2.0/Microsoft.Agents.AI.dll + true + CP0005 M:Microsoft.Agents.AI.AgentSkillScript.RunAsync(Microsoft.Agents.AI.AgentSkill,System.Nullable{System.Text.Json.JsonElement},System.IServiceProvider,System.Threading.CancellationToken) diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkill.cs b/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkill.cs index 6f549301d0..38342d6816 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkill.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkill.cs @@ -1,7 +1,8 @@ // Copyright (c) Microsoft. All rights reserved. -using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.Threading; +using System.Threading.Tasks; using Microsoft.Shared.DiagnosticIds; namespace Microsoft.Agents.AI; @@ -34,29 +35,44 @@ public abstract class AgentSkill /// /// Gets the full skill content. /// - /// + /// Cancellation token. + /// /// For file-based skills this is the raw SKILL.md file content, optionally /// augmented with a synthesized scripts block when scripts are present. /// For code-defined skills this is a synthesized XML document /// containing name, description, and body (instructions, resources, scripts). - /// - public abstract string Content { get; } + /// + public abstract ValueTask GetContentAsync(CancellationToken cancellationToken = default); /// - /// Gets the resources associated with this skill, or if none. + /// Gets a resource owned by this skill by name. /// + /// The resource name (e.g. an identifier or a relative path referenced inside the skill content). + /// Cancellation token. + /// + /// The , or when no resource with the given name exists. + /// /// - /// The default implementation returns . - /// Override this property in derived classes to provide skill-specific resources. + /// The default implementation returns . Override in derived classes that + /// expose resources. /// - public virtual IReadOnlyList? Resources => null; + public virtual ValueTask GetResourceAsync( + string name, + CancellationToken cancellationToken = default) => default; /// - /// Gets the scripts associated with this skill, or if none. + /// Gets a script owned by this skill by name. /// + /// The script name. + /// Cancellation token. + /// + /// The , or when no script with the given name exists. + /// /// - /// The default implementation returns . - /// Override this property in derived classes to provide skill-specific scripts. + /// The default implementation returns . Override in derived classes that + /// expose scripts. /// - public virtual IReadOnlyList? Scripts => null; + public virtual ValueTask GetScriptAsync( + string name, + CancellationToken cancellationToken = default) => default; } diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProvider.cs b/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProvider.cs index af1225c9df..4d9ae154f0 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProvider.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProvider.cs @@ -186,13 +186,10 @@ public sealed partial class AgentSkillsProvider : AIContextProvider return await base.ProvideAIContextAsync(context, cancellationToken).ConfigureAwait(false); } - bool hasScripts = skills.Any(s => s.Scripts is { Count: > 0 }); - bool hasResources = skills.Any(s => s.Resources is { Count: > 0 }); - return new AIContext { - Instructions = this.BuildSkillsInstructions(skills, includeScriptInstructions: hasScripts, hasResources), - Tools = this.BuildTools(skills, hasScripts, hasResources), + Instructions = this.BuildSkillsInstructions(skills), + Tools = this.BuildTools(skills), }; } @@ -219,29 +216,20 @@ public sealed partial class AgentSkillsProvider : AIContextProvider } } - private IList BuildTools(IList skills, bool hasScripts, bool hasResources) + private IList BuildTools(IList skills) { IList tools = [ AIFunctionFactory.Create( - (string skillName) => this.LoadSkill(skills, skillName), + (string skillName, CancellationToken cancellationToken) => this.LoadSkillAsync(skills, skillName, cancellationToken), name: "load_skill", description: "Loads the full content of a specific skill"), - ]; - - if (hasResources) - { - tools.Add(AIFunctionFactory.Create( + AIFunctionFactory.Create( (string skillName, string resourceName, IServiceProvider? serviceProvider, CancellationToken cancellationToken = default) => this.ReadSkillResourceAsync(skills, skillName, resourceName, serviceProvider, cancellationToken), name: "read_skill_resource", - description: "Reads a resource associated with a skill, such as references, assets, or dynamic data.")); - } - - if (!hasScripts) - { - return tools; - } + description: "Reads a resource associated with a skill, such as references, assets, or dynamic data."), + ]; AIFunction scriptFunction = AIFunctionFactory.Create( (string skillName, string scriptName, JsonElement? arguments = null, IServiceProvider? serviceProvider = null, CancellationToken cancellationToken = default) => @@ -257,7 +245,7 @@ public sealed partial class AgentSkillsProvider : AIContextProvider return [.. tools, scriptFunction]; } - private string? BuildSkillsInstructions(IList skills, bool includeScriptInstructions, bool includeResourceInstructions) + private string? BuildSkillsInstructions(IList skills) { string promptTemplate = this._options?.SkillsInstructionPrompt ?? DefaultSkillsInstructionPrompt; @@ -270,32 +258,29 @@ public sealed partial class AgentSkillsProvider : AIContextProvider sb.AppendLine(" "); } - string resourceInstruction = includeResourceInstructions - ? """ + const string ResourceInstruction = + """ - Use `read_skill_resource` to read any referenced resources, using the name exactly as listed (e.g. `"style-guide"` not `"style-guide.md"`, `"references/FAQ.md"` not `"FAQ.md"`). - """ - : string.Empty; + """; - string scriptInstruction = includeScriptInstructions - ? "- Use `run_skill_script` to run referenced scripts, using the name exactly as listed." - : string.Empty; + const string ScriptInstruction = "- Use `run_skill_script` to run referenced scripts, using the name exactly as listed."; return new StringBuilder(promptTemplate) .Replace(SkillsPlaceholder, sb.ToString().TrimEnd()) - .Replace(ResourceInstructionsPlaceholder, resourceInstruction) - .Replace(ScriptInstructionsPlaceholder, scriptInstruction) + .Replace(ResourceInstructionsPlaceholder, ResourceInstruction) + .Replace(ScriptInstructionsPlaceholder, ScriptInstruction) .ToString(); } - private string LoadSkill(IList skills, string skillName) + private async Task LoadSkillAsync(IList skills, string skillName, CancellationToken cancellationToken) { if (string.IsNullOrWhiteSpace(skillName)) { return "Error: Skill name cannot be empty."; } - var skill = skills?.FirstOrDefault(skill => skill.Frontmatter.Name == skillName); + var skill = skills.FirstOrDefault(skill => skill.Frontmatter.Name == skillName); if (skill == null) { return $"Error: Skill '{skillName}' not found."; @@ -303,7 +288,7 @@ public sealed partial class AgentSkillsProvider : AIContextProvider LogSkillLoading(this._logger, skillName); - return skill.Content; + return await skill.GetContentAsync(cancellationToken).ConfigureAwait(false); } private async Task ReadSkillResourceAsync(IList skills, string skillName, string resourceName, IServiceProvider? serviceProvider, CancellationToken cancellationToken = default) @@ -318,20 +303,20 @@ public sealed partial class AgentSkillsProvider : AIContextProvider return "Error: Resource name cannot be empty."; } - var skill = skills?.FirstOrDefault(skill => skill.Frontmatter.Name == skillName); + var skill = skills.FirstOrDefault(skill => skill.Frontmatter.Name == skillName); if (skill == null) { return $"Error: Skill '{skillName}' not found."; } - var resource = skill.Resources?.FirstOrDefault(resource => resource.Name == resourceName); - if (resource is null) - { - return $"Error: Resource '{resourceName}' not found in skill '{skillName}'."; - } - try { + var resource = await skill.GetResourceAsync(resourceName, cancellationToken).ConfigureAwait(false); + if (resource is null) + { + return $"Error: Resource '{resourceName}' not found in skill '{skillName}'."; + } + return await resource.ReadAsync(serviceProvider, cancellationToken).ConfigureAwait(false); } catch (Exception ex) @@ -353,20 +338,20 @@ public sealed partial class AgentSkillsProvider : AIContextProvider return "Error: Script name cannot be empty."; } - var skill = skills?.FirstOrDefault(skill => skill.Frontmatter.Name == skillName); + var skill = skills.FirstOrDefault(skill => skill.Frontmatter.Name == skillName); if (skill == null) { return $"Error: Skill '{skillName}' not found."; } - var script = skill.Scripts?.FirstOrDefault(resource => resource.Name == scriptName); - if (script is null) - { - return $"Error: Script '{scriptName}' not found in skill '{skillName}'."; - } - try { + var script = await skill.GetScriptAsync(scriptName, cancellationToken).ConfigureAwait(false); + if (script is null) + { + return $"Error: Script '{scriptName}' not found in skill '{skillName}'."; + } + return await script.RunAsync(skill, arguments, serviceProvider, cancellationToken).ConfigureAwait(false); } catch (Exception ex) diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkill.cs b/dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkill.cs index 3e10557968..d1c792de21 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkill.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkill.cs @@ -2,6 +2,9 @@ using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; using Microsoft.Shared.DiagnosticIds; using Microsoft.Shared.Diagnostics; @@ -50,11 +53,12 @@ public sealed class AgentFileSkill : AgentSkill /// block is appended with a per-script entry describing the expected argument format. /// The result is cached after the first access. /// - public override string Content + public override ValueTask GetContentAsync(CancellationToken cancellationToken = default) { - get => this._content ??= this._scripts is { Count: > 0 } + var content = this._content ??= this._scripts is { Count: > 0 } ? this._originalContent + AgentInlineSkillContentBuilder.BuildScriptsBlock(this._scripts) : this._originalContent; + return new(content); } /// @@ -63,8 +67,16 @@ public sealed class AgentFileSkill : AgentSkill public string Path { get; } /// - public override IReadOnlyList Resources => this._resources; + public override ValueTask GetResourceAsync(string name, CancellationToken cancellationToken = default) + { + var resource = this._resources.FirstOrDefault(r => r.Name == name); + return new(resource); + } /// - public override IReadOnlyList Scripts => this._scripts; + public override ValueTask GetScriptAsync(string name, CancellationToken cancellationToken = default) + { + var script = this._scripts.FirstOrDefault(s => s.Name == name); + return new(script); + } } diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentClassSkill.cs b/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentClassSkill.cs index b44f423bc2..32f461e32a 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentClassSkill.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentClassSkill.cs @@ -4,9 +4,11 @@ using System; using System.Collections.Generic; using System.ComponentModel; using System.Diagnostics.CodeAnalysis; +using System.Linq; using System.Reflection; using System.Text.Json; using System.Threading; +using System.Threading.Tasks; using Microsoft.Extensions.AI; using Microsoft.Shared.DiagnosticIds; @@ -34,9 +36,9 @@ namespace Microsoft.Agents.AI; /// discovered via reflection on . This approach is compatible with Native AOT. /// /// -/// Explicit override: Override and , using -/// , , -/// and to define inline resources and scripts. This approach is also compatible with Native AOT. +/// Explicit override: Override and , using , +/// , and to define +/// inline resources and scripts. This approach is also compatible with Native AOT. /// /// /// @@ -97,11 +99,24 @@ public abstract class AgentClassSkill< { private const BindingFlags DiscoveryBindingFlags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static; - private string? _content; - private bool _resourcesDiscovered; - private bool _scriptsDiscovered; - private IReadOnlyList? _reflectedResources; - private IReadOnlyList? _reflectedScripts; + private readonly Lazy?> _resources; + private readonly Lazy?> _scripts; + private readonly Lazy _content; + + /// + /// Initializes a new instance of the class. + /// + protected AgentClassSkill() + { + this._resources = new Lazy?>(this.DiscoverResources); + this._scripts = new Lazy?>(this.DiscoverScripts); + this._content = new Lazy(() => AgentInlineSkillContentBuilder.Build( + this.Frontmatter.Name, + this.Frontmatter.Description, + this.Instructions, + this.Resources, + this.Scripts)); + } /// /// Gets the raw instructions text for this skill. @@ -126,53 +141,44 @@ public abstract class AgentClassSkill< /// Returns a synthesized XML document containing name, description, instructions, resources, and scripts. /// The result is cached after the first access. Override to provide custom content. /// - public override string Content => this._content ??= AgentInlineSkillContentBuilder.Build( - this.Frontmatter.Name, - this.Frontmatter.Description, - this.Instructions, - this.Resources, - this.Scripts); + public override ValueTask GetContentAsync(CancellationToken cancellationToken = default) => new(this._content.Value); + + /// + /// Gets the resources associated with this skill, or if none. + /// + /// + /// The default implementation returns resources discovered via reflection by scanning + /// for members annotated with . + /// This discovery is compatible with Native AOT because is annotated with + /// . The result is cached after the first access. + /// Override this property in derived classes to provide skill-specific resources. + /// + public virtual IReadOnlyList? Resources => this._resources.Value; + + /// + /// Gets the scripts associated with this skill, or if none. + /// + /// + /// The default implementation returns scripts discovered via reflection by scanning + /// for methods annotated with . + /// This discovery is compatible with Native AOT because is annotated with + /// . The result is cached after the first access. + /// Override this property in derived classes to provide skill-specific scripts. + /// + public virtual IReadOnlyList? Scripts => this._scripts.Value; /// - /// - /// Returns resources discovered via reflection by scanning for - /// members annotated with . This discovery is - /// compatible with Native AOT because is annotated with - /// . The result is cached after the first access. - /// - public override IReadOnlyList? Resources + public sealed override ValueTask GetResourceAsync(string name, CancellationToken cancellationToken = default) { - get - { - if (!this._resourcesDiscovered) - { - this._reflectedResources = this.DiscoverResources(); - this._resourcesDiscovered = true; - } - - return this._reflectedResources; - } + var resource = this.Resources?.FirstOrDefault(r => r.Name == name); + return new(resource); } /// - /// - /// Returns scripts discovered via reflection by scanning for - /// methods annotated with . This discovery is - /// compatible with Native AOT because is annotated with - /// . The result is cached after the first access. - /// - public override IReadOnlyList? Scripts + public sealed override ValueTask GetScriptAsync(string name, CancellationToken cancellationToken = default) { - get - { - if (!this._scriptsDiscovered) - { - this._reflectedScripts = this.DiscoverScripts(); - this._scriptsDiscovered = true; - } - - return this._reflectedScripts; - } + var script = this.Scripts?.FirstOrDefault(s => s.Name == name); + return new(script); } /// diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentInlineSkill.cs b/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentInlineSkill.cs index cdfb14a584..4fb2b045cb 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentInlineSkill.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentInlineSkill.cs @@ -3,7 +3,10 @@ using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.Linq; using System.Text.Json; +using System.Threading; +using System.Threading.Tasks; using Microsoft.Extensions.AI; using Microsoft.Shared.DiagnosticIds; using Microsoft.Shared.Diagnostics; @@ -16,9 +19,9 @@ namespace Microsoft.Agents.AI; /// /// All calls to , /// , and -/// must be made before the skill's is first accessed. +/// must be made before the skill's is first called. /// Calls made after that point will not be reflected in the generated -/// . In typical usage, this means configuring all +/// content. In typical usage, this means configuring all /// resources and scripts before registering the skill with an /// or . /// @@ -90,13 +93,24 @@ public sealed class AgentInlineSkill : AgentSkill public override AgentSkillFrontmatter Frontmatter { get; } /// - public override string Content => this._cachedContent ??= AgentInlineSkillContentBuilder.Build(this.Frontmatter.Name, this.Frontmatter.Description, this._instructions, this._resources, this._scripts); + public override ValueTask GetContentAsync(CancellationToken cancellationToken = default) + { + return new(this._cachedContent ??= AgentInlineSkillContentBuilder.Build(this.Frontmatter.Name, this.Frontmatter.Description, this._instructions, this._resources, this._scripts)); + } /// - public override IReadOnlyList? Resources => this._resources; + public override ValueTask GetResourceAsync(string name, CancellationToken cancellationToken = default) + { + var resource = this._resources?.FirstOrDefault(r => r.Name == name); + return new(resource); + } /// - public override IReadOnlyList? Scripts => this._scripts; + public override ValueTask GetScriptAsync(string name, CancellationToken cancellationToken = default) + { + var script = this._scripts?.FirstOrDefault(s => s.Name == name); + return new(script); + } /// /// Registers a static resource with this skill. diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentSkillResourceAttribute.cs b/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentSkillResourceAttribute.cs index a642d6c281..7094ab57ce 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentSkillResourceAttribute.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentSkillResourceAttribute.cs @@ -27,7 +27,7 @@ namespace Microsoft.Agents.AI; /// /// /// This attribute is compatible with Native AOT when used with . -/// Alternatively, override the property and use +/// Alternatively, override and use /// instead. /// /// diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentSkillScriptAttribute.cs b/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentSkillScriptAttribute.cs index 30f65cf383..afb76c108d 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentSkillScriptAttribute.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentSkillScriptAttribute.cs @@ -26,7 +26,7 @@ namespace Microsoft.Agents.AI; /// /// /// This attribute is compatible with Native AOT when used with . -/// Alternatively, override the property and use +/// Alternatively, override and use /// instead. /// /// diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentClassSkillTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentClassSkillTests.cs index dc83fad119..1248866a52 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentClassSkillTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentClassSkillTests.cs @@ -18,7 +18,7 @@ namespace Microsoft.Agents.AI.UnitTests.AgentSkills; public sealed class AgentClassSkillTests { [Fact] - public void MinimalClassSkill_HasNullOverrides_AndSynthesizesContent() + public async Task MinimalClassSkill_HasNullOverrides_AndSynthesizesContentAsync() { // Arrange var skill = new MinimalClassSkill(); @@ -26,18 +26,17 @@ public sealed class AgentClassSkillTests // Act & Assert — null overrides Assert.Equal("minimal", skill.Frontmatter.Name); Assert.Null(skill.Resources); - Assert.Null(skill.Scripts); // Act & Assert — synthesized XML content - Assert.Contains("minimal", skill.Content); - Assert.Contains("A minimal skill.", skill.Content); - Assert.Contains("", skill.Content); - Assert.Contains("Minimal skill body.", skill.Content); - Assert.Contains("", skill.Content); + Assert.Contains("minimal", await skill.GetContentAsync()); + Assert.Contains("A minimal skill.", await skill.GetContentAsync()); + Assert.Contains("", await skill.GetContentAsync()); + Assert.Contains("Minimal skill body.", await skill.GetContentAsync()); + Assert.Contains("", await skill.GetContentAsync()); } [Fact] - public void FullClassSkill_ReturnsOverriddenLists_AndCachesContent() + public async Task FullClassSkill_ReturnsOverriddenLists_AndCachesContentAsync() { // Arrange var skill = new FullClassSkill(); @@ -50,11 +49,11 @@ public sealed class AgentClassSkillTests Assert.Equal("TestScript", skill.Scripts![0].Name); // Act & Assert — Content is cached - Assert.Same(skill.Content, skill.Content); + Assert.Same(await skill.GetContentAsync(), await skill.GetContentAsync()); // Act & Assert — Content includes parameter schema from typed script - Assert.Contains("parameters_schema", skill.Content); - Assert.Contains("value", skill.Content); + Assert.Contains("parameters_schema", await skill.GetContentAsync()); + Assert.Contains("value", await skill.GetContentAsync()); } [Fact] @@ -117,6 +116,116 @@ public sealed class AgentClassSkillTests Assert.Single(scriptOnly.Scripts!); } + [Fact] + public async Task GetResourceAsync_ExistingName_ReturnsResourceAsync() + { + // Arrange + var skill = new FullClassSkill(); + + // Act + var resource = await skill.GetResourceAsync("test-resource"); + + // Assert + Assert.NotNull(resource); + Assert.Equal("test-resource", resource!.Name); + } + + [Fact] + public async Task GetResourceAsync_NonExistingName_ReturnsNullAsync() + { + // Arrange + var skill = new FullClassSkill(); + + // Act + var resource = await skill.GetResourceAsync("missing"); + + // Assert + Assert.Null(resource); + } + + [Fact] + public async Task GetResourceAsync_NoResources_ReturnsNullAsync() + { + // Arrange + var skill = new MinimalClassSkill(); + + // Act + var resource = await skill.GetResourceAsync("anything"); + + // Assert + Assert.Null(resource); + } + + [Fact] + public async Task GetScriptAsync_ExistingName_ReturnsScriptAsync() + { + // Arrange + var skill = new FullClassSkill(); + + // Act + var script = await skill.GetScriptAsync("TestScript"); + + // Assert + Assert.NotNull(script); + Assert.Equal("TestScript", script!.Name); + } + + [Fact] + public async Task GetScriptAsync_NonExistingName_ReturnsNullAsync() + { + // Arrange + var skill = new FullClassSkill(); + + // Act + var script = await skill.GetScriptAsync("missing"); + + // Assert + Assert.Null(script); + } + + [Fact] + public async Task GetScriptAsync_NoScripts_ReturnsNullAsync() + { + // Arrange + var skill = new MinimalClassSkill(); + + // Act + var script = await skill.GetScriptAsync("anything"); + + // Assert + Assert.Null(script); + } + + [Fact] + public async Task ConcurrentAccess_ToReflectedResourcesScriptsAndContent_InvokesDiscoveryOnceAsync() + { + // Regression test for thread-safety of Lazy initialization in AgentClassSkill. + // AttributedFullSkill uses attribute-based discovery (no override), so it exercises + // the base class's Lazy fields rather than a subclass's own caching. + var skill = new AttributedFullSkill(); + + const int Concurrency = 32; + var resourcesResults = new IReadOnlyList?[Concurrency]; + var scriptsResults = new IReadOnlyList?[Concurrency]; + var contentResults = new string[Concurrency]; + + // Act — invoke all three accessors concurrently from many threads. + await Task.WhenAll(Enumerable.Range(0, Concurrency).Select(i => Task.Run(async () => + { + resourcesResults[i] = skill.Resources; + scriptsResults[i] = skill.Scripts; + contentResults[i] = await skill.GetContentAsync(); + }))); + + // Assert — every thread observed the same cached instances (no torn state). + for (int i = 1; i < Concurrency; i++) + { + Assert.Same(resourcesResults[0], resourcesResults[i]); + Assert.Same(scriptsResults[0], scriptsResults[i]); + Assert.Same(contentResults[0], contentResults[i]); + } + } + [Fact] public async Task CreateScriptAndResource_WithSerializerOptions_HandleCustomTypesAsync() { @@ -151,17 +260,14 @@ public sealed class AgentClassSkillTests // Arrange var skill = new AttributedScriptsSkill(); - // Act - var scripts = skill.Scripts; + // Act & Assert — all scripts discovered with correct metadata + Assert.NotNull(skill.Scripts); + Assert.Equal(4, skill.Scripts!.Count); + Assert.Contains(skill.Scripts, s => s.Name == "do-work"); + Assert.Contains(skill.Scripts, s => s.Name == "DefaultNamed"); + Assert.Contains(skill.Scripts, s => s.Name == "append"); - // Assert — all scripts discovered with correct metadata - Assert.NotNull(scripts); - Assert.Equal(4, scripts!.Count); - Assert.Contains(scripts, s => s.Name == "do-work"); - Assert.Contains(scripts, s => s.Name == "DefaultNamed"); - Assert.Contains(scripts, s => s.Name == "append"); - - var processScript = scripts.First(s => s.Name == "process"); + var processScript = skill.Scripts.First(s => s.Name == "process"); Assert.Equal("Processes the input.", processScript.Description); } @@ -272,16 +378,16 @@ public sealed class AgentClassSkillTests } [Fact] - public void AttributedFullSkill_IncludesContentWithSchema_AndCachesMembers() + public async Task AttributedFullSkill_IncludesContentWithSchema_AndCachesMembersAsync() { // Arrange var skill = new AttributedFullSkill(); // Act & Assert — Content includes reflected resources and scripts - Assert.Contains("", skill.Content); - Assert.Contains("conversion-table", skill.Content); - Assert.Contains("", skill.Content); - Assert.Contains("convert", skill.Content); + Assert.Contains("", await skill.GetContentAsync()); + Assert.Contains("conversion-table", await skill.GetContentAsync()); + Assert.Contains("", await skill.GetContentAsync()); + Assert.Contains("convert", await skill.GetContentAsync()); // Act & Assert — discovered members are cached Assert.Same(skill.Resources, skill.Resources); @@ -299,33 +405,33 @@ public sealed class AgentClassSkillTests // Arrange — skill with no attributes and no overrides; base discovery returns null (not empty list) var skill = new NoAttributesNoOverridesSkill(); var baseType = typeof(AgentClassSkill); - var resourcesDiscoveredField = baseType.GetField("_resourcesDiscovered", BindingFlags.Instance | BindingFlags.NonPublic); - var scriptsDiscoveredField = baseType.GetField("_scriptsDiscovered", BindingFlags.Instance | BindingFlags.NonPublic); - var reflectedResourcesField = baseType.GetField("_reflectedResources", BindingFlags.Instance | BindingFlags.NonPublic); - var reflectedScriptsField = baseType.GetField("_reflectedScripts", BindingFlags.Instance | BindingFlags.NonPublic); + var resourcesField = baseType.GetField("_resources", BindingFlags.Instance | BindingFlags.NonPublic); + var scriptsField = baseType.GetField("_scripts", BindingFlags.Instance | BindingFlags.NonPublic); - Assert.NotNull(resourcesDiscoveredField); - Assert.NotNull(scriptsDiscoveredField); - Assert.NotNull(reflectedResourcesField); - Assert.NotNull(reflectedScriptsField); - Assert.False((bool)resourcesDiscoveredField!.GetValue(skill)!); - Assert.False((bool)scriptsDiscoveredField!.GetValue(skill)!); + Assert.NotNull(resourcesField); + Assert.NotNull(scriptsField); + + var resourcesLazy = (Lazy?>)resourcesField!.GetValue(skill)!; + var scriptsLazy = (Lazy?>)scriptsField!.GetValue(skill)!; + + Assert.False(resourcesLazy.IsValueCreated); + Assert.False(scriptsLazy.IsValueCreated); // Act & Assert Assert.Null(skill.Resources); Assert.Null(skill.Scripts); - Assert.True((bool)resourcesDiscoveredField.GetValue(skill)!); - Assert.True((bool)scriptsDiscoveredField.GetValue(skill)!); - Assert.Null(reflectedResourcesField!.GetValue(skill)); - Assert.Null(reflectedScriptsField!.GetValue(skill)); + Assert.True(resourcesLazy.IsValueCreated); + Assert.True(scriptsLazy.IsValueCreated); + Assert.Null(resourcesLazy.Value); + Assert.Null(scriptsLazy.Value); // Repeated access should not re-trigger discovery even when discovered value is null. Assert.Null(skill.Resources); Assert.Null(skill.Scripts); - Assert.True((bool)resourcesDiscoveredField.GetValue(skill)!); - Assert.True((bool)scriptsDiscoveredField.GetValue(skill)!); - Assert.Null(reflectedResourcesField.GetValue(skill)); - Assert.Null(reflectedScriptsField.GetValue(skill)); + Assert.True(resourcesLazy.IsValueCreated); + Assert.True(scriptsLazy.IsValueCreated); + Assert.Null(resourcesLazy.Value); + Assert.Null(scriptsLazy.Value); } [Fact] @@ -382,7 +488,7 @@ public sealed class AgentClassSkillTests var jso = SkillTestJsonContext.Default.Options; // Act & Assert — script with custom JSO - var script = skill.Scripts![0]; + var script = skill.Scripts!.First(s => s.Name == "lookup"); var inputJson = JsonSerializer.SerializeToElement(new LookupRequest { Query = "test", MaxResults = 3 }, jso); using var argsDoc = JsonDocument.Parse($$"""{ "request": {{inputJson.GetRawText()}} }"""); var args = argsDoc.RootElement; @@ -398,13 +504,13 @@ public sealed class AgentClassSkillTests } [Fact] - public void Content_IncludesDescription_ForReflectedResources() + public async Task Content_IncludesDescription_ForReflectedResourcesAsync() { // Arrange var skill = new AttributedResourcePropertiesSkill(); // Act - var content = skill.Content; + var content = await skill.GetContentAsync(); // Assert — descriptions from [Description] attribute appear in synthesized content Assert.Contains("Some important data.", content); diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillScriptTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillScriptTests.cs index e638380019..9a27528051 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillScriptTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillScriptTests.cs @@ -105,7 +105,7 @@ public sealed class AgentFileSkillScriptTests } [Fact] - public void Content_WithScripts_AppendsPerScriptEntries() + public async Task Content_WithScripts_AppendsPerScriptEntriesAsync() { // Arrange static Task RunnerAsync(AgentFileSkill s, AgentFileSkillScript sc, JsonElement? a, IServiceProvider? sp, CancellationToken ct) => Task.FromResult(null); @@ -118,7 +118,7 @@ public sealed class AgentFileSkillScriptTests scripts: [script1, script2]); // Act - var content = fileSkill.Content; + var content = await fileSkill.GetContentAsync(); // Assert — content starts with original and appends per-script entries Assert.StartsWith("Original content", content); @@ -130,7 +130,7 @@ public sealed class AgentFileSkillScriptTests } [Fact] - public void Content_WithoutScripts_ReturnsOriginalContent() + public async Task Content_WithoutScripts_ReturnsOriginalContentAsync() { // Arrange var fileSkill = new AgentFileSkill( @@ -139,14 +139,14 @@ public sealed class AgentFileSkillScriptTests "/skills/my-skill"); // Act - var content = fileSkill.Content; + var content = await fileSkill.GetContentAsync(); // Assert Assert.Equal("Original content only", content); } [Fact] - public void Content_WithScripts_IsCached() + public async Task Content_WithScripts_IsCachedAsync() { // Arrange static Task RunnerAsync(AgentFileSkill s, AgentFileSkillScript sc, JsonElement? a, IServiceProvider? sp, CancellationToken ct) => Task.FromResult(null); @@ -158,8 +158,8 @@ public sealed class AgentFileSkillScriptTests scripts: [script]); // Act - var content1 = fileSkill.Content; - var content2 = fileSkill.Content; + var content1 = await fileSkill.GetContentAsync(); + var content2 = await fileSkill.GetContentAsync(); // Assert Assert.Same(content1, content2); @@ -232,7 +232,7 @@ public sealed class AgentFileSkillScriptTests } [Fact] - public void Content_WithScripts_ContainsDefaultParametersSchema() + public async Task Content_WithScripts_ContainsDefaultParametersSchemaAsync() { // Arrange static Task RunnerAsync(AgentFileSkill s, AgentFileSkillScript sc, JsonElement? a, IServiceProvider? sp, CancellationToken ct) => Task.FromResult(null); @@ -244,7 +244,7 @@ public sealed class AgentFileSkillScriptTests scripts: [script]); // Act - var content = fileSkill.Content; + var content = await fileSkill.GetContentAsync(); // Assert — the appended block contains the actual default schema from AgentFileSkillScript Assert.Contains("""{"type":"array","items":{"type":"string"}}""", content); diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillsSourceScriptTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillsSourceScriptTests.cs index 7b4dfa1812..32e018ae90 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillsSourceScriptTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillsSourceScriptTests.cs @@ -2,7 +2,6 @@ using System; using System.IO; -using System.Linq; using System.Text.Json; using System.Threading; using System.Threading.Tasks; @@ -46,9 +45,9 @@ public sealed class AgentFileSkillsSourceScriptTests : IDisposable // Assert Assert.Single(skills); var skill = skills[0]; - Assert.NotNull(skill.Scripts); - Assert.Single(skill.Scripts!); - Assert.Equal("scripts/convert.py", skill.Scripts![0].Name); + var script = await skill.GetScriptAsync("scripts/convert.py"); + Assert.NotNull(script); + Assert.Equal("scripts/convert.py", script!.Name); } [Fact] @@ -69,14 +68,13 @@ public sealed class AgentFileSkillsSourceScriptTests : IDisposable // Assert Assert.Single(skills); - var scriptNames = skills[0].Scripts!.Select(s => s.Name).OrderBy(n => n, StringComparer.Ordinal).ToList(); - Assert.Equal(6, scriptNames.Count); - Assert.Contains("scripts/run.cs", scriptNames); - Assert.Contains("scripts/run.csx", scriptNames); - Assert.Contains("scripts/run.js", scriptNames); - Assert.Contains("scripts/run.ps1", scriptNames); - Assert.Contains("scripts/run.py", scriptNames); - Assert.Contains("scripts/run.sh", scriptNames); + // Assert — verify all expected scripts are discoverable + foreach (var name in (string[])["scripts/run.cs", "scripts/run.csx", "scripts/run.js", "scripts/run.ps1", "scripts/run.py", "scripts/run.sh"]) + { + var script = await skills[0].GetScriptAsync(name); + Assert.NotNull(script); + Assert.Equal(name, script!.Name); + } } [Fact] @@ -94,7 +92,7 @@ public sealed class AgentFileSkillsSourceScriptTests : IDisposable // Assert Assert.Single(skills); - Assert.Empty(skills[0].Scripts!); + Assert.Null(await skills[0].GetScriptAsync("scripts/data.txt")); } [Fact] @@ -109,8 +107,7 @@ public sealed class AgentFileSkillsSourceScriptTests : IDisposable // Assert Assert.Single(skills); - Assert.NotNull(skills[0].Scripts); - Assert.Empty(skills[0].Scripts!); + Assert.Null(await skills[0].GetScriptAsync("any-script")); } [Fact] @@ -128,7 +125,7 @@ public sealed class AgentFileSkillsSourceScriptTests : IDisposable // Assert — neither file is in the default scripts/ directory, so no scripts are discovered Assert.Single(skills); - Assert.Empty(skills[0].Scripts!); + Assert.Null(await skills[0].GetScriptAsync("convert.py")); } [Fact] @@ -150,7 +147,7 @@ public sealed class AgentFileSkillsSourceScriptTests : IDisposable // Act var skills = await source.GetSkillsAsync(CancellationToken.None); - var scriptResult = await skills[0].Scripts![0].RunAsync(skills[0], null, null, CancellationToken.None); + var scriptResult = await (await skills[0].GetScriptAsync("scripts/test.py"))!.RunAsync(skills[0], null, null, CancellationToken.None); // Assert Assert.True(executorCalled); @@ -175,7 +172,7 @@ public sealed class AgentFileSkillsSourceScriptTests : IDisposable // Act — discovery succeeds even without a runner var skills = await source.GetSkillsAsync(CancellationToken.None); - var script = skills[0].Scripts![0]; + var script = (await skills[0].GetScriptAsync("scripts/run.sh"))!; // Assert — running the script throws because no runner was provided await Assert.ThrowsAsync(() => script.RunAsync(skills[0], null, null, CancellationToken.None)); @@ -195,8 +192,9 @@ public sealed class AgentFileSkillsSourceScriptTests : IDisposable // Assert Assert.Single(skills); - Assert.Single(skills[0].Scripts!); - Assert.Equal("scripts/run.rb", skills[0].Scripts![0].Name); + var rbScript = await skills[0].GetScriptAsync("scripts/run.rb"); + Assert.NotNull(rbScript); + Assert.Equal("scripts/run.rb", rbScript!.Name); } [Fact] @@ -217,7 +215,7 @@ public sealed class AgentFileSkillsSourceScriptTests : IDisposable var skills = await source.GetSkillsAsync(CancellationToken.None); using var argumentsDoc = JsonDocument.Parse("""{"value":26.2,"factor":1.60934}"""); var arguments = argumentsDoc.RootElement; - await skills[0].Scripts![0].RunAsync(skills[0], arguments, null, CancellationToken.None); + await (await skills[0].GetScriptAsync("scripts/test.py"))!.RunAsync(skills[0], arguments, null, CancellationToken.None); // Assert Assert.NotNull(capturedArgs); @@ -240,8 +238,9 @@ public sealed class AgentFileSkillsSourceScriptTests : IDisposable // Assert — script file inside the deeply nested directory is discovered Assert.Single(skills); - Assert.Single(skills[0].Scripts!); - Assert.Equal("f1/f2/f3/run.py", skills[0].Scripts![0].Name); + var nestedScript = await skills[0].GetScriptAsync("f1/f2/f3/run.py"); + Assert.NotNull(nestedScript); + Assert.Equal("f1/f2/f3/run.py", nestedScript!.Name); } [Theory] @@ -267,11 +266,12 @@ public sealed class AgentFileSkillsSourceScriptTests : IDisposable // Assert — scripts are discovered with names identical to using directories without "./" Assert.Single(skills); - Assert.Equal(directories.Length, skills[0].Scripts!.Count); foreach (string directory in directories) { string expectedName = $"{directory.Substring(2)}/run.py"; - Assert.Contains(skills[0].Scripts!, s => s.Name == expectedName); + var script = await skills[0].GetScriptAsync(expectedName); + Assert.NotNull(script); + Assert.Equal(expectedName, script!.Name); } } diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentInlineSkillTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentInlineSkillTests.cs index 817223bac7..c6ae2212bf 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentInlineSkillTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentInlineSkillTests.cs @@ -105,13 +105,13 @@ public sealed class AgentInlineSkillTests } [Fact] - public void Content_ContainsNameDescriptionAndInstructions() + public async Task Content_ContainsNameDescriptionAndInstructionsAsync() { // Arrange var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Do the thing."); // Act - var content = skill.Content; + var content = await skill.GetContentAsync(); // Assert Assert.Contains("my-skill", content); @@ -120,13 +120,13 @@ public sealed class AgentInlineSkillTests } [Fact] - public void Content_EscapesXmlCharacters() + public async Task Content_EscapesXmlCharactersAsync() { // Arrange var skill = new AgentInlineSkill("my-skill", "xz\"w & it's more", "1 & 2 < 3"); // Act - var content = skill.Content; + var content = await skill.GetContentAsync(); // Assert Assert.Contains("my-skill", content); @@ -135,28 +135,28 @@ public sealed class AgentInlineSkillTests } [Fact] - public void Content_IsCachedAcrossAccesses() + public async Task Content_IsCachedAcrossAccessesAsync() { // Arrange var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); // Act - var first = skill.Content; - var second = skill.Content; + var first = await skill.GetContentAsync(); + var second = await skill.GetContentAsync(); // Assert Assert.Same(first, second); } [Fact] - public void Content_IncludesResourcesAddedBeforeFirstAccess() + public async Task Content_IncludesResourcesAddedBeforeFirstAccessAsync() { // Arrange var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); skill.AddResource("config", "value1", "A config resource."); // Act - var content = skill.Content; + var content = await skill.GetContentAsync(); // Assert Assert.Contains("", content); @@ -164,14 +164,14 @@ public sealed class AgentInlineSkillTests } [Fact] - public void Content_IncludesDelegateResourcesAddedBeforeFirstAccess() + public async Task Content_IncludesDelegateResourcesAddedBeforeFirstAccessAsync() { // Arrange var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); skill.AddResource("dynamic", () => "hello"); // Act - var content = skill.Content; + var content = await skill.GetContentAsync(); // Assert Assert.Contains("", content); @@ -179,14 +179,14 @@ public sealed class AgentInlineSkillTests } [Fact] - public void Content_IncludesScriptsAddedBeforeFirstAccess() + public async Task Content_IncludesScriptsAddedBeforeFirstAccessAsync() { // Arrange var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); skill.AddScript("run", () => "result", "Runs something."); // Act - var content = skill.Content; + var content = await skill.GetContentAsync(); // Assert Assert.Contains("", content); @@ -194,22 +194,22 @@ public sealed class AgentInlineSkillTests } [Fact] - public void Content_IsCachedAndNotRebuilt() + public async Task Content_IsCachedAndNotRebuiltAsync() { // Arrange var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); skill.AddResource("r1", "v1"); // Act - var first = skill.Content; - var second = skill.Content; + var first = await skill.GetContentAsync(); + var second = await skill.GetContentAsync(); // Assert Assert.Same(first, second); } [Fact] - public void Content_IncludesResourcesAndScriptsAddedBeforeFirstAccess() + public async Task Content_IncludesResourcesAndScriptsAddedBeforeFirstAccessAsync() { // Arrange var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); @@ -217,7 +217,7 @@ public sealed class AgentInlineSkillTests skill.AddScript("s1", () => "ok"); // Act - var content = skill.Content; + var content = await skill.GetContentAsync(); // Assert Assert.Contains("", content); @@ -227,14 +227,14 @@ public sealed class AgentInlineSkillTests } [Fact] - public void Content_ParametersSchema_IsXmlEscaped() + public async Task Content_ParametersSchema_IsXmlEscapedAsync() { // Arrange var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); skill.AddScript("search", (string query, int limit) => $"found {limit} results for {query}"); // Act - var content = skill.Content; + var content = await skill.GetContentAsync(); // Assert — JSON schema should be present and XML content chars escaped Assert.Contains("parameters_schema", content); @@ -280,17 +280,103 @@ public sealed class AgentInlineSkillTests var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); // Act & Assert - Assert.Null(skill.Resources); + Assert.Null(skill.GetTestResources()); } [Fact] - public void Scripts_WhenNoneAdded_ReturnsNull() + public async Task Scripts_WhenNoneAdded_ReturnsNullAsync() { // Arrange var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); // Act & Assert - Assert.Null(skill.Scripts); + Assert.Null(await skill.GetScriptAsync("nonexistent")); + } + + [Fact] + public async Task GetResourceAsync_ExistingName_ReturnsResourceAsync() + { + // Arrange + var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); + skill.AddResource("r1", "v1"); + skill.AddResource("r2", "v2"); + + // Act + var resource = await skill.GetResourceAsync("r2"); + + // Assert + Assert.NotNull(resource); + Assert.Equal("r2", resource!.Name); + } + + [Fact] + public async Task GetResourceAsync_NonExistingName_ReturnsNullAsync() + { + // Arrange + var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); + skill.AddResource("r1", "v1"); + + // Act + var resource = await skill.GetResourceAsync("missing"); + + // Assert + Assert.Null(resource); + } + + [Fact] + public async Task GetResourceAsync_NoResourcesAdded_ReturnsNullAsync() + { + // Arrange + var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); + + // Act + var resource = await skill.GetResourceAsync("missing"); + + // Assert + Assert.Null(resource); + } + + [Fact] + public async Task GetScriptAsync_ExistingName_ReturnsScriptAsync() + { + // Arrange + var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); + skill.AddScript("s1", () => "first"); + skill.AddScript("s2", () => "second"); + + // Act + var script = await skill.GetScriptAsync("s2"); + + // Assert + Assert.NotNull(script); + Assert.Equal("s2", script!.Name); + } + + [Fact] + public async Task GetScriptAsync_NonExistingName_ReturnsNullAsync() + { + // Arrange + var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); + skill.AddScript("s1", () => "ok"); + + // Act + var script = await skill.GetScriptAsync("missing"); + + // Assert + Assert.Null(script); + } + + [Fact] + public async Task GetScriptAsync_NoScriptsAdded_ReturnsNullAsync() + { + // Arrange + var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); + + // Act + var script = await skill.GetScriptAsync("missing"); + + // Assert + Assert.Null(script); } [Fact] @@ -333,13 +419,13 @@ public sealed class AgentInlineSkillTests } [Fact] - public void Content_NoResourcesOrScripts_DoesNotContainResourcesOrScriptsTags() + public async Task Content_NoResourcesOrScripts_DoesNotContainResourcesOrScriptsTagsAsync() { // Arrange var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); // Act - var content = skill.Content; + var content = await skill.GetContentAsync(); // Assert Assert.DoesNotContain("", content); @@ -347,58 +433,58 @@ public sealed class AgentInlineSkillTests } [Fact] - public void Content_ResourcesAddedAfterCaching_AreNotIncluded() + public async Task Content_ResourcesAddedAfterCaching_AreNotIncludedAsync() { // Arrange var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); - _ = skill.Content; // trigger caching + _ = await skill.GetContentAsync(); // trigger caching skill.AddResource("late-resource", "late-value"); // Act - var content = skill.Content; + var content = await skill.GetContentAsync(); // Assert — the late resource should not appear because content was cached Assert.DoesNotContain("late-resource", content); } [Fact] - public void Content_ScriptsAddedAfterCaching_AreNotIncluded() + public async Task Content_ScriptsAddedAfterCaching_AreNotIncludedAsync() { // Arrange var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); - _ = skill.Content; // trigger caching + _ = await skill.GetContentAsync(); // trigger caching skill.AddScript("late-script", () => "late"); // Act - var content = skill.Content; + var content = await skill.GetContentAsync(); // Assert — the late script should not appear because content was cached Assert.DoesNotContain("late-script", content); } [Fact] - public void Content_ScriptWithDescription_IncludesDescriptionAttribute() + public async Task Content_ScriptWithDescription_IncludesDescriptionAttributeAsync() { // Arrange var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); skill.AddScript("my-script", () => "ok", "Runs something."); // Act - var content = skill.Content; + var content = await skill.GetContentAsync(); // Assert Assert.Contains("description=\"Runs something.\"", content); } [Fact] - public void Content_ScriptWithoutParametersOrDescription_UsesSelfClosingTag() + public async Task Content_ScriptWithoutParametersOrDescription_UsesSelfClosingTagAsync() { // Arrange var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); skill.AddScript("simple", () => "ok"); // Act - var content = skill.Content; + var content = await skill.GetContentAsync(); // Assert — parameterless Action delegates still produce a schema, so this // verifies the script is at least included in the output @@ -406,7 +492,7 @@ public sealed class AgentInlineSkillTests } [Fact] - public void Content_ResourceWithDescription_IncludesDescriptionAttribute() + public async Task Content_ResourceWithDescription_IncludesDescriptionAttributeAsync() { // Arrange var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); @@ -414,7 +500,7 @@ public sealed class AgentInlineSkillTests skill.AddResource("no-desc", "value"); // Act - var content = skill.Content; + var content = await skill.GetContentAsync(); // Assert Assert.Contains("description=\"A described resource.\"", content); @@ -437,7 +523,7 @@ public sealed class AgentInlineSkillTests var args = argsDoc.RootElement; // Act - var result = await skill.Scripts![0].RunAsync(skill, args, null, CancellationToken.None); + var result = await (await skill.GetScriptAsync("lookup"))!.RunAsync(skill, args, null, CancellationToken.None); // Assert — the custom input was deserialized via skill-level JSO and response was produced Assert.NotNull(result); @@ -461,7 +547,7 @@ public sealed class AgentInlineSkillTests var args = argsDoc.RootElement; // Act - var result = await skill.Scripts![0].RunAsync(skill, args, null, CancellationToken.None); + var result = await (await skill.GetScriptAsync("lookup"))!.RunAsync(skill, args, null, CancellationToken.None); // Assert — per-script JSO takes effect and custom types are properly marshaled Assert.NotNull(result); @@ -477,7 +563,7 @@ public sealed class AgentInlineSkillTests skill.AddResource("config", () => new SkillConfig { Theme = "dark", Verbose = true }); // Act - var result = await skill.Resources![0].ReadAsync(); + var result = await skill.GetTestResources()![0].ReadAsync(); // Assert — the custom type was returned successfully via skill-level JSO Assert.NotNull(result); @@ -494,7 +580,7 @@ public sealed class AgentInlineSkillTests skill.AddResource("config", () => new SkillConfig { Theme = "dark", Verbose = true }, serializerOptions: resourceJso); // Act - var result = await skill.Resources![0].ReadAsync(); + var result = await skill.GetTestResources()![0].ReadAsync(); // Assert — per-resource JSO takes effect and custom type is properly marshaled Assert.NotNull(result); diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillTestExtensions.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillTestExtensions.cs new file mode 100644 index 0000000000..5a67bcf62c --- /dev/null +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillTestExtensions.cs @@ -0,0 +1,43 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.Collections.Generic; +using System.Linq; +using System.Reflection; + +namespace Microsoft.Agents.AI.UnitTests.AgentSkills; + +/// +/// Test-only helpers that peek at the underlying resource list of a skill via reflection. +/// +/// +/// The public API exposes resources only through +/// . +/// These helpers exist purely to allow unit tests for and +/// to inspect the concrete enumerated list a skill carries. +/// +internal static class AgentSkillTestExtensions +{ + public static IReadOnlyList? GetTestResources(this AgentSkill skill) + { + // AgentFileSkill / AgentInlineSkill: private "_resources" field. + for (var type = skill.GetType(); type is not null; type = type.BaseType) + { + var field = type.GetField("_resources", BindingFlags.NonPublic | BindingFlags.Instance); + if (field is not null) + { + return UnwrapList(field.GetValue(skill)); + } + } + + return null; + } + + private static IReadOnlyList? UnwrapList(object? value) => + value switch + { + null => null, + IReadOnlyList list => list, + IEnumerable seq => seq.ToList(), + _ => null, + }; +} diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillsProviderTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillsProviderTests.cs index 123be17387..25481ea501 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillsProviderTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillsProviderTests.cs @@ -68,11 +68,12 @@ public sealed class AgentSkillsProviderTests : IDisposable Assert.Contains("provider-skill", result.Instructions); Assert.Contains("Provider skill test", result.Instructions); - // Should have load_skill tool (no resources, so no read_skill_resource) + // Should have load_skill, read_skill_resource, and run_skill_script tools Assert.NotNull(result.Tools); var toolNames = result.Tools!.Select(t => t.Name).ToList(); Assert.Contains("load_skill", toolNames); - Assert.DoesNotContain("read_skill_resource", toolNames); + Assert.Contains("read_skill_resource", toolNames); + Assert.Contains("run_skill_script", toolNames); } [Fact] @@ -316,7 +317,7 @@ public sealed class AgentSkillsProviderTests : IDisposable } [Fact] - public async Task InvokingCoreAsync_WithoutScripts_NoRunSkillScriptToolAsync() + public async Task InvokingCoreAsync_WithoutScripts_StillIncludesAllToolsAsync() { // Arrange this.CreateSkill("no-script-skill", "No scripts", "Body."); @@ -328,10 +329,12 @@ public sealed class AgentSkillsProviderTests : IDisposable // Act var result = await provider.InvokingAsync(invokingContext, CancellationToken.None); - // Assert + // Assert — all tools are always included regardless of skill content Assert.NotNull(result.Tools); var toolNames = result.Tools!.Select(t => t.Name).ToList(); - Assert.DoesNotContain("run_skill_script", toolNames); + Assert.Contains("load_skill", toolNames); + Assert.Contains("read_skill_resource", toolNames); + Assert.Contains("run_skill_script", toolNames); } [Fact] @@ -416,7 +419,7 @@ public sealed class AgentSkillsProviderTests : IDisposable // Assert Assert.Single(skills); var fileSkill = Assert.IsType(skills[0]); - Assert.All(fileSkill.Resources, r => Assert.EndsWith(".json", r.Name)); + Assert.All(fileSkill.GetTestResources()!, r => Assert.EndsWith(".json", r.Name)); } private void CreateSkill(string name, string description, string body) @@ -445,6 +448,279 @@ public sealed class AgentSkillsProviderTests : IDisposable Assert.Contains("Skill body.", text); } + [Fact] + public async Task LoadSkill_EmptySkillName_ReturnsErrorAsync() + { + // Arrange + this.CreateSkill("any-skill", "Test", "Body."); + var provider = new AgentSkillsProvider(new AgentFileSkillsSource(this._testRoot, s_noOpExecutor)); + var invokingContext = new AIContextProvider.InvokingContext(this._agent, session: null, new AIContext()); + var result = await provider.InvokingAsync(invokingContext, CancellationToken.None); + var loadSkillTool = result.Tools!.First(t => t.Name == "load_skill") as AIFunction; + + // Act + var content = await loadSkillTool!.InvokeAsync(new AIFunctionArguments(new Dictionary { ["skillName"] = "" })); + + // Assert + Assert.Equal("Error: Skill name cannot be empty.", content!.ToString()); + } + + [Fact] + public async Task LoadSkill_SkillNotFound_ReturnsErrorAsync() + { + // Arrange + this.CreateSkill("only-skill", "Test", "Body."); + var provider = new AgentSkillsProvider(new AgentFileSkillsSource(this._testRoot, s_noOpExecutor)); + var invokingContext = new AIContextProvider.InvokingContext(this._agent, session: null, new AIContext()); + var result = await provider.InvokingAsync(invokingContext, CancellationToken.None); + var loadSkillTool = result.Tools!.First(t => t.Name == "load_skill") as AIFunction; + + // Act + var content = await loadSkillTool!.InvokeAsync(new AIFunctionArguments(new Dictionary { ["skillName"] = "non-existent" })); + + // Assert + Assert.Equal("Error: Skill 'non-existent' not found.", content!.ToString()); + } + + [Fact] + public async Task InvokingCoreAsync_WithResources_IncludesReadSkillResourceToolAsync() + { + // Arrange — inline skill with a resource + var skill = new AgentInlineSkill("res-skill", "Has resources", "Body."); + skill.AddResource("config", "value1", "A config resource."); + var provider = new AgentSkillsProvider(skill); + var invokingContext = new AIContextProvider.InvokingContext(this._agent, session: null, new AIContext()); + + // Act + var result = await provider.InvokingAsync(invokingContext, CancellationToken.None); + + // Assert + Assert.NotNull(result.Tools); + var toolNames = result.Tools!.Select(t => t.Name).ToList(); + Assert.Contains("read_skill_resource", toolNames); + } + + [Fact] + public async Task ReadSkillResource_ReturnsResourceContentAsync() + { + // Arrange — inline skill with a resource + var skill = new AgentInlineSkill("res-skill", "Has resources", "Body."); + skill.AddResource("config", "resource-value"); + var provider = new AgentSkillsProvider(skill); + var invokingContext = new AIContextProvider.InvokingContext(this._agent, session: null, new AIContext()); + var result = await provider.InvokingAsync(invokingContext, CancellationToken.None); + var readTool = result.Tools!.First(t => t.Name == "read_skill_resource") as AIFunction; + + // Act + var content = await readTool!.InvokeAsync(new AIFunctionArguments(new Dictionary + { + ["skillName"] = "res-skill", + ["resourceName"] = "config", + }) + { + Services = new TestServiceProvider(), + }); + + // Assert + Assert.Equal("resource-value", content!.ToString()); + } + + [Fact] + public async Task ReadSkillResource_EmptySkillName_ReturnsErrorAsync() + { + // Arrange + var skill = new AgentInlineSkill("res-skill", "Has resources", "Body."); + skill.AddResource("config", "v"); + var provider = new AgentSkillsProvider(skill); + var invokingContext = new AIContextProvider.InvokingContext(this._agent, session: null, new AIContext()); + var result = await provider.InvokingAsync(invokingContext, CancellationToken.None); + var readTool = result.Tools!.First(t => t.Name == "read_skill_resource") as AIFunction; + + // Act + var content = await readTool!.InvokeAsync(new AIFunctionArguments(new Dictionary + { + ["skillName"] = "", + ["resourceName"] = "config", + }) + { + Services = new TestServiceProvider(), + }); + + // Assert + Assert.Equal("Error: Skill name cannot be empty.", content!.ToString()); + } + + [Fact] + public async Task ReadSkillResource_EmptyResourceName_ReturnsErrorAsync() + { + // Arrange + var skill = new AgentInlineSkill("res-skill", "Has resources", "Body."); + skill.AddResource("config", "v"); + var provider = new AgentSkillsProvider(skill); + var invokingContext = new AIContextProvider.InvokingContext(this._agent, session: null, new AIContext()); + var result = await provider.InvokingAsync(invokingContext, CancellationToken.None); + var readTool = result.Tools!.First(t => t.Name == "read_skill_resource") as AIFunction; + + // Act + var content = await readTool!.InvokeAsync(new AIFunctionArguments(new Dictionary + { + ["skillName"] = "res-skill", + ["resourceName"] = "", + }) + { + Services = new TestServiceProvider(), + }); + + // Assert + Assert.Equal("Error: Resource name cannot be empty.", content!.ToString()); + } + + [Fact] + public async Task ReadSkillResource_SkillNotFound_ReturnsErrorAsync() + { + // Arrange + var skill = new AgentInlineSkill("res-skill", "Has resources", "Body."); + skill.AddResource("config", "v"); + var provider = new AgentSkillsProvider(skill); + var invokingContext = new AIContextProvider.InvokingContext(this._agent, session: null, new AIContext()); + var result = await provider.InvokingAsync(invokingContext, CancellationToken.None); + var readTool = result.Tools!.First(t => t.Name == "read_skill_resource") as AIFunction; + + // Act + var content = await readTool!.InvokeAsync(new AIFunctionArguments(new Dictionary + { + ["skillName"] = "non-existent", + ["resourceName"] = "config", + }) + { + Services = new TestServiceProvider(), + }); + + // Assert + Assert.Equal("Error: Skill 'non-existent' not found.", content!.ToString()); + } + + [Fact] + public async Task ReadSkillResource_ResourceNotFound_ReturnsErrorAsync() + { + // Arrange + var skill = new AgentInlineSkill("res-skill", "Has resources", "Body."); + skill.AddResource("config", "v"); + var provider = new AgentSkillsProvider(skill); + var invokingContext = new AIContextProvider.InvokingContext(this._agent, session: null, new AIContext()); + var result = await provider.InvokingAsync(invokingContext, CancellationToken.None); + var readTool = result.Tools!.First(t => t.Name == "read_skill_resource") as AIFunction; + + // Act + var content = await readTool!.InvokeAsync(new AIFunctionArguments(new Dictionary + { + ["skillName"] = "res-skill", + ["resourceName"] = "missing", + }) + { + Services = new TestServiceProvider(), + }); + + // Assert + Assert.Equal("Error: Resource 'missing' not found in skill 'res-skill'.", content!.ToString()); + } + + [Fact] + public async Task RunSkillScript_EmptySkillName_ReturnsErrorAsync() + { + // Arrange + string skillDir = Path.Combine(this._testRoot, "err-script-skill"); + Directory.CreateDirectory(Path.Combine(skillDir, "scripts")); + File.WriteAllText(Path.Combine(skillDir, "SKILL.md"), "---\nname: err-script-skill\ndescription: Test\n---\nBody."); + File.WriteAllText(Path.Combine(skillDir, "scripts", "run.py"), "print('hi')"); + var provider = new AgentSkillsProvider(new AgentFileSkillsSource(this._testRoot, s_noOpExecutor)); + var invokingContext = new AIContextProvider.InvokingContext(this._agent, session: null, new AIContext()); + var result = await provider.InvokingAsync(invokingContext, CancellationToken.None); + var runScriptTool = result.Tools!.First(t => t.Name == "run_skill_script") as AIFunction; + + // Act + var content = await runScriptTool!.InvokeAsync(new AIFunctionArguments(new Dictionary + { + ["skillName"] = "", + ["scriptName"] = "scripts/run.py", + })); + + // Assert + Assert.Equal("Error: Skill name cannot be empty.", content!.ToString()); + } + + [Fact] + public async Task RunSkillScript_EmptyScriptName_ReturnsErrorAsync() + { + // Arrange + string skillDir = Path.Combine(this._testRoot, "err-script2-skill"); + Directory.CreateDirectory(Path.Combine(skillDir, "scripts")); + File.WriteAllText(Path.Combine(skillDir, "SKILL.md"), "---\nname: err-script2-skill\ndescription: Test\n---\nBody."); + File.WriteAllText(Path.Combine(skillDir, "scripts", "run.py"), "print('hi')"); + var provider = new AgentSkillsProvider(new AgentFileSkillsSource(this._testRoot, s_noOpExecutor)); + var invokingContext = new AIContextProvider.InvokingContext(this._agent, session: null, new AIContext()); + var result = await provider.InvokingAsync(invokingContext, CancellationToken.None); + var runScriptTool = result.Tools!.First(t => t.Name == "run_skill_script") as AIFunction; + + // Act + var content = await runScriptTool!.InvokeAsync(new AIFunctionArguments(new Dictionary + { + ["skillName"] = "err-script2-skill", + ["scriptName"] = "", + })); + + // Assert + Assert.Equal("Error: Script name cannot be empty.", content!.ToString()); + } + + [Fact] + public async Task RunSkillScript_SkillNotFound_ReturnsErrorAsync() + { + // Arrange + string skillDir = Path.Combine(this._testRoot, "err-script3-skill"); + Directory.CreateDirectory(Path.Combine(skillDir, "scripts")); + File.WriteAllText(Path.Combine(skillDir, "SKILL.md"), "---\nname: err-script3-skill\ndescription: Test\n---\nBody."); + File.WriteAllText(Path.Combine(skillDir, "scripts", "run.py"), "print('hi')"); + var provider = new AgentSkillsProvider(new AgentFileSkillsSource(this._testRoot, s_noOpExecutor)); + var invokingContext = new AIContextProvider.InvokingContext(this._agent, session: null, new AIContext()); + var result = await provider.InvokingAsync(invokingContext, CancellationToken.None); + var runScriptTool = result.Tools!.First(t => t.Name == "run_skill_script") as AIFunction; + + // Act + var content = await runScriptTool!.InvokeAsync(new AIFunctionArguments(new Dictionary + { + ["skillName"] = "non-existent", + ["scriptName"] = "scripts/run.py", + })); + + // Assert + Assert.Equal("Error: Skill 'non-existent' not found.", content!.ToString()); + } + + [Fact] + public async Task RunSkillScript_ScriptNotFound_ReturnsErrorAsync() + { + // Arrange + string skillDir = Path.Combine(this._testRoot, "err-script4-skill"); + Directory.CreateDirectory(Path.Combine(skillDir, "scripts")); + File.WriteAllText(Path.Combine(skillDir, "SKILL.md"), "---\nname: err-script4-skill\ndescription: Test\n---\nBody."); + File.WriteAllText(Path.Combine(skillDir, "scripts", "run.py"), "print('hi')"); + var provider = new AgentSkillsProvider(new AgentFileSkillsSource(this._testRoot, s_noOpExecutor)); + var invokingContext = new AIContextProvider.InvokingContext(this._agent, session: null, new AIContext()); + var result = await provider.InvokingAsync(invokingContext, CancellationToken.None); + var runScriptTool = result.Tools!.First(t => t.Name == "run_skill_script") as AIFunction; + + // Act + var content = await runScriptTool!.InvokeAsync(new AIFunctionArguments(new Dictionary + { + ["skillName"] = "err-script4-skill", + ["scriptName"] = "scripts/missing.py", + })); + + // Assert + Assert.Equal("Error: Script 'scripts/missing.py' not found in skill 'err-script4-skill'.", content!.ToString()); + } + [Fact] public async Task Builder_UseFileScriptRunnerAfterUseFileSkills_RunnerIsUsedAsync() { @@ -998,9 +1274,5 @@ public sealed class AgentSkillsProviderTests : IDisposable public override AgentSkillFrontmatter Frontmatter { get; } protected override string Instructions => this._instructions; - - public override IReadOnlyList? Resources => null; - - public override IReadOnlyList? Scripts => null; } } diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs index 731258a90b..cd362ce64e 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs @@ -281,9 +281,9 @@ public sealed class FileAgentSkillLoaderTests : IDisposable // Assert Assert.Single(skills); var skill = skills[0]; - Assert.Equal(2, skill.Resources!.Count); - Assert.Contains(skill.Resources!, r => r.Name.Equals("references/FAQ.md", StringComparison.OrdinalIgnoreCase)); - Assert.Contains(skill.Resources!, r => r.Name.Equals("assets/data.json", StringComparison.OrdinalIgnoreCase)); + Assert.Equal(2, skill.GetTestResources()!.Count); + Assert.Contains(skill.GetTestResources()!, r => r.Name.Equals("references/FAQ.md", StringComparison.OrdinalIgnoreCase)); + Assert.Contains(skill.GetTestResources()!, r => r.Name.Equals("assets/data.json", StringComparison.OrdinalIgnoreCase)); } [Fact] @@ -306,8 +306,8 @@ public sealed class FileAgentSkillLoaderTests : IDisposable // Assert Assert.Single(skills); var skill = skills[0]; - Assert.Single(skill.Resources!); - Assert.Equal("references/data.json", skill.Resources![0].Name); + Assert.Single(skill.GetTestResources()!); + Assert.Equal("references/data.json", skill.GetTestResources()![0].Name); } [Fact] @@ -329,8 +329,8 @@ public sealed class FileAgentSkillLoaderTests : IDisposable // Assert Assert.Single(skills); var skill = skills[0]; - Assert.Single(skill.Resources!); - Assert.Equal("references/notes.md", skill.Resources![0].Name); + Assert.Single(skill.GetTestResources()!); + Assert.Equal("references/notes.md", skill.GetTestResources()![0].Name); } [Fact] @@ -355,9 +355,9 @@ public sealed class FileAgentSkillLoaderTests : IDisposable // Assert — only the file directly in references/ is discovered; the nested file is not Assert.Single(skills); var skill = skills[0]; - Assert.Single(skill.Resources!); - Assert.Contains(skill.Resources!, r => r.Name.Equals("references/top.md", StringComparison.OrdinalIgnoreCase)); - Assert.DoesNotContain(skill.Resources!, r => r.Name.Contains("deep.md", StringComparison.OrdinalIgnoreCase)); + Assert.Single(skill.GetTestResources()!); + Assert.Contains(skill.GetTestResources()!, r => r.Name.Equals("references/top.md", StringComparison.OrdinalIgnoreCase)); + Assert.DoesNotContain(skill.GetTestResources()!, r => r.Name.Contains("deep.md", StringComparison.OrdinalIgnoreCase)); } [Fact] @@ -380,8 +380,8 @@ public sealed class FileAgentSkillLoaderTests : IDisposable // Assert — only .custom files should be discovered, not .json Assert.Single(skills); var skill = skills[0]; - Assert.Single(skill.Resources!); - Assert.Equal("references/data.custom", skill.Resources![0].Name); + Assert.Single(skill.GetTestResources()!); + Assert.Equal("references/data.custom", skill.GetTestResources()![0].Name); } [Theory] @@ -406,7 +406,7 @@ public sealed class FileAgentSkillLoaderTests : IDisposable // Assert — default extensions include .md var skills = await source.GetSkillsAsync(); - Assert.Single(skills[0].Resources!); + Assert.Single(skills[0].GetTestResources()!); } [Fact] @@ -442,7 +442,7 @@ public sealed class FileAgentSkillLoaderTests : IDisposable // Assert — root-level files are NOT discovered unless "." is in ResourceDirectories Assert.Single(skills); - Assert.Empty(skills[0].Resources!); + Assert.Empty(skills[0].GetTestResources()!); } [Fact] @@ -465,9 +465,9 @@ public sealed class FileAgentSkillLoaderTests : IDisposable // Assert — both root-level resource files (and SKILL.md excluded) should be discovered Assert.Single(skills); var skill = skills[0]; - Assert.Equal(2, skill.Resources!.Count); - Assert.Contains(skill.Resources!, r => r.Name.Equals("guide.md", StringComparison.OrdinalIgnoreCase)); - Assert.Contains(skill.Resources!, r => r.Name.Equals("config.json", StringComparison.OrdinalIgnoreCase)); + Assert.Equal(2, skill.GetTestResources()!.Count); + Assert.Contains(skill.GetTestResources()!, r => r.Name.Equals("guide.md", StringComparison.OrdinalIgnoreCase)); + Assert.Contains(skill.GetTestResources()!, r => r.Name.Equals("config.json", StringComparison.OrdinalIgnoreCase)); } [Fact] @@ -488,7 +488,7 @@ public sealed class FileAgentSkillLoaderTests : IDisposable // Assert — non-spec directories are not scanned by default Assert.Single(skills); - Assert.Empty(skills[0].Resources!); + Assert.Empty(skills[0].GetTestResources()!); } [Fact] @@ -514,8 +514,8 @@ public sealed class FileAgentSkillLoaderTests : IDisposable // Assert — only docs/ is scanned; references/ is NOT scanned Assert.Single(skills); var skill = skills[0]; - Assert.Single(skill.Resources!); - Assert.Equal("docs/readme.md", skill.Resources![0].Name); + Assert.Single(skill.GetTestResources()!); + Assert.Equal("docs/readme.md", skill.GetTestResources()![0].Name); } [Fact] @@ -530,7 +530,7 @@ public sealed class FileAgentSkillLoaderTests : IDisposable // Assert Assert.Single(skills); - Assert.Empty(skills[0].Resources!); + Assert.Empty(skills[0].GetTestResources()!); } [Fact] @@ -588,7 +588,7 @@ public sealed class FileAgentSkillLoaderTests : IDisposable File.WriteAllText(Path.Combine(refsDir, "doc.md"), "Document content here."); var source = new AgentFileSkillsSource(this._testRoot, s_noOpExecutor); var skills = await source.GetSkillsAsync(); - var resource = skills[0].Resources!.First(r => r.Name == "references/doc.md"); + var resource = skills[0].GetTestResources()!.First(r => r.Name == "references/doc.md"); // Act var content = await resource.ReadAsync(); @@ -672,8 +672,8 @@ public sealed class FileAgentSkillLoaderTests : IDisposable // Assert — skill should still load, the symlinked references/ is skipped, assets/legit.md is found var skill = skills.FirstOrDefault(s => s.Frontmatter.Name == "symlink-escape-skill"); Assert.NotNull(skill); - Assert.Single(skill.Resources!); - Assert.Equal("assets/legit.md", skill.Resources![0].Name); + Assert.Single(skill.GetTestResources()!); + Assert.Equal("assets/legit.md", skill.GetTestResources()![0].Name); } [Fact] @@ -714,8 +714,8 @@ public sealed class FileAgentSkillLoaderTests : IDisposable // Assert — only assets/legit.md is found; the symlinked references/ directory is skipped entirely var skill = skills.FirstOrDefault(s => s.Frontmatter.Name == "symlink-directory-skip"); Assert.NotNull(skill); - Assert.Single(skill.Resources!); - Assert.Equal("assets/legit.md", skill.Resources![0].Name); + Assert.Single(skill.GetTestResources()!); + Assert.Equal("assets/legit.md", skill.GetTestResources()![0].Name); } [Fact] @@ -751,7 +751,7 @@ public sealed class FileAgentSkillLoaderTests : IDisposable // Assert — skill loads but scripts from the symlinked directory are not discovered var skill = skills.FirstOrDefault(s => s.Frontmatter.Name == "symlink-script-skip"); Assert.NotNull(skill); - Assert.Empty(skill.Scripts!); + Assert.Null(await skill.GetScriptAsync("any-script")); } [Fact] @@ -791,7 +791,7 @@ public sealed class FileAgentSkillLoaderTests : IDisposable // Assert — the symlinked intermediate segment causes the directory to be skipped var skill = skills.FirstOrDefault(s => s.Frontmatter.Name == "symlink-intermediate"); Assert.NotNull(skill); - Assert.Empty(skill.Resources!); + Assert.Empty(skill.GetTestResources()!); } #endif @@ -1020,8 +1020,8 @@ public sealed class FileAgentSkillLoaderTests : IDisposable // Assert — only one copy of the resource despite two equivalent directory entries Assert.Single(skills); - Assert.Single(skills[0].Resources!); - Assert.Equal("references/FAQ.md", skills[0].Resources![0].Name); + Assert.Single(skills[0].GetTestResources()!); + Assert.Equal("references/FAQ.md", skills[0].GetTestResources()![0].Name); } [Fact] @@ -1043,8 +1043,8 @@ public sealed class FileAgentSkillLoaderTests : IDisposable // Assert — trailing slash variant deduplicated Assert.Single(skills); - Assert.Single(skills[0].Resources!); - Assert.Equal("references/data.json", skills[0].Resources![0].Name); + Assert.Single(skills[0].GetTestResources()!); + Assert.Equal("references/data.json", skills[0].GetTestResources()![0].Name); } [Fact] @@ -1066,8 +1066,9 @@ public sealed class FileAgentSkillLoaderTests : IDisposable // Assert — backslash variant deduplicated Assert.Single(skills); - Assert.Single(skills[0].Scripts!); - Assert.Equal("scripts/run.py", skills[0].Scripts![0].Name); + var script = await skills[0].GetScriptAsync("scripts/run.py"); + Assert.NotNull(script); + Assert.Equal("scripts/run.py", script!.Name); } [Theory] @@ -1093,8 +1094,8 @@ public sealed class FileAgentSkillLoaderTests : IDisposable // Assert — the resource is discovered with a name identical to using the directory without "./" Assert.Single(skills); - Assert.Single(skills[0].Resources!); - Assert.Equal($"{directoryWithoutDotSlash}/data.json", skills[0].Resources![0].Name); + Assert.Single(skills[0].GetTestResources()!); + Assert.Equal($"{directoryWithoutDotSlash}/data.json", skills[0].GetTestResources()![0].Name); } [Fact] @@ -1117,8 +1118,8 @@ public sealed class FileAgentSkillLoaderTests : IDisposable // Assert — resource file inside the deeply nested directory is discovered Assert.Single(skills); var skill = skills[0]; - Assert.Single(skill.Resources!); - Assert.Equal("f1/f2/f3/data.json", skill.Resources![0].Name); + Assert.Single(skill.GetTestResources()!); + Assert.Equal("f1/f2/f3/data.json", skill.GetTestResources()![0].Name); } private string CreateSkillDirectory(string name, string description, string body) @@ -1188,8 +1189,9 @@ public sealed class FileAgentSkillLoaderTests : IDisposable // Assert — script at the skill root should be discovered var skill = skills.FirstOrDefault(s => s.Frontmatter.Name == "root-script-skill"); Assert.NotNull(skill); - Assert.Single(skill.Scripts!); - Assert.Equal("run.py", skill.Scripts![0].Name); + var script = await skill.GetScriptAsync("run.py"); + Assert.NotNull(script); + Assert.Equal("run.py", script!.Name); } #if NET @@ -1229,8 +1231,8 @@ public sealed class FileAgentSkillLoaderTests : IDisposable // Assert — only legit.md should be discovered; the symlinked leak.md is skipped var skill = skills.FirstOrDefault(s => s.Frontmatter.Name == "symlink-file-skill"); Assert.NotNull(skill); - Assert.Single(skill.Resources!); - Assert.Equal("references/legit.md", skill.Resources![0].Name); + Assert.Single(skill.GetTestResources()!); + Assert.Equal("references/legit.md", skill.GetTestResources()![0].Name); } #endif } diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/HostedAgentSkillsPatternTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/HostedAgentSkillsPatternTests.cs index d566f1db6f..76cdaa8569 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/HostedAgentSkillsPatternTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/HostedAgentSkillsPatternTests.cs @@ -155,9 +155,9 @@ public sealed class HostedAgentSkillsPatternTests : IDisposable Assert.NotNull(result.Tools); var toolNames = result.Tools!.Select(t => t.Name).ToList(); Assert.Contains("load_skill", toolNames); - // No scripts or resources => no read_skill_resource or run_skill_script - Assert.DoesNotContain("read_skill_resource", toolNames); - Assert.DoesNotContain("run_skill_script", toolNames); + // All tools are always included regardless of whether skills have resources or scripts + Assert.Contains("read_skill_resource", toolNames); + Assert.Contains("run_skill_script", toolNames); } [Fact] diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/TestSkillTypes.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/TestSkillTypes.cs index a2fb4155e4..7110fb438b 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/TestSkillTypes.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/TestSkillTypes.cs @@ -31,13 +31,7 @@ internal sealed class TestAgentSkill : AgentSkill public override AgentSkillFrontmatter Frontmatter => this._frontmatter; /// - public override string Content => this._content; - - /// - public override IReadOnlyList? Resources => null; - - /// - public override IReadOnlyList? Scripts => null; + public override ValueTask GetContentAsync(CancellationToken cancellationToken = default) => new(this._content); } ///