From 6a9c6a95d1f7e47eed4f7f53b9147a8881b7e9e4 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Fri, 22 May 2026 14:13:48 +0100 Subject: [PATCH 1/7] .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> --- .../CompatibilitySuppressions.xml | 140 +++++++++ .../Microsoft.Agents.AI/Skills/AgentSkill.cs | 56 +++- .../Skills/AgentSkillsProvider.cs | 30 +- .../Skills/File/AgentFileSkill.cs | 26 +- .../Skills/Programmatic/AgentClassSkill.cs | 102 ++++--- .../Skills/Programmatic/AgentInlineSkill.cs | 30 +- .../AgentSkillResourceAttribute.cs | 2 +- .../Programmatic/AgentSkillScriptAttribute.cs | 2 +- .../AgentSkills/AgentClassSkillTests.cs | 251 ++++++++++++---- .../AgentSkills/AgentFileSkillScriptTests.cs | 20 +- .../AgentFileSkillsSourceScriptTests.cs | 54 ++-- .../AgentSkills/AgentInlineSkillTests.cs | 200 ++++++++++--- .../AgentSkills/AgentSkillTestExtensions.cs | 57 ++++ .../AgentSkills/AgentSkillsProviderTests.cs | 279 +++++++++++++++++- .../AgentSkills/FileAgentSkillLoaderTests.cs | 88 +++--- .../AgentSkills/TestSkillTypes.cs | 7 +- 16 files changed, 1083 insertions(+), 261 deletions(-) create mode 100644 dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillTestExtensions.cs 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..48a6142d54 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,62 @@ 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 ValueTask GetContentAsync(CancellationToken cancellationToken = default); + + /// + /// Gets a value indicating whether this skill may have resources that can be served via . + /// + /// + /// The default implementation returns . For skills whose resource + /// availability cannot be determined in advance (e.g. MCP skills), this should return + /// as a conservative default. + /// + public virtual bool HasResources => false; + + /// + /// Gets a value indicating whether this skill may have scripts that can be served via . + /// + /// + /// The default implementation returns . /// - public abstract string Content { get; } + public virtual bool HasScripts => false; /// - /// 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, and set to . /// - 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, and set to . /// - 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..048ba36416 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProvider.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProvider.cs @@ -186,8 +186,8 @@ private async Task CreateContextAsync(InvokingContext context, Cancel 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 }); + bool hasScripts = skills.Any(s => s.HasScripts); + bool hasResources = skills.Any(s => s.HasResources); return new AIContext { @@ -224,7 +224,7 @@ private IList BuildTools(IList skills, bool hasScripts, 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"), ]; @@ -288,14 +288,14 @@ private IList BuildTools(IList skills, bool hasScripts, .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 +303,7 @@ private string LoadSkill(IList skills, string skillName) 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 +318,20 @@ private string LoadSkill(IList skills, string skillName) 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,13 +353,13 @@ private string LoadSkill(IList skills, string skillName) 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); + var script = await skill.GetScriptAsync(scriptName, cancellationToken).ConfigureAwait(false); if (script is null) { return $"Error: Script '{scriptName}' not found in skill '{skillName}'."; diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkill.cs b/dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkill.cs index 3e10557968..e0712cfe25 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 @@ internal AgentFileSkill( /// 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,22 @@ public override string Content public string Path { get; } /// - public override IReadOnlyList Resources => this._resources; + public override bool HasResources => this._resources.Count > 0; /// - public override IReadOnlyList Scripts => this._scripts; + public override bool HasScripts => this._scripts.Count > 0; + + /// + public override ValueTask GetResourceAsync(string name, CancellationToken cancellationToken = default) + { + var resource = this._resources.FirstOrDefault(r => r.Name == name); + return new(resource); + } + + /// + 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..1d706b6f4e 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.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,50 @@ 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. + /// /// - /// Returns resources discovered via reflection by scanning for - /// members annotated with . This discovery is - /// compatible with Native AOT because is annotated with + /// 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 override IReadOnlyList? Resources - { - get - { - if (!this._resourcesDiscovered) - { - this._reflectedResources = this.DiscoverResources(); - this._resourcesDiscovered = true; - } + public virtual IReadOnlyList? Resources => this._resources.Value; - return this._reflectedResources; - } - } - - /// + /// + /// Gets the scripts associated with this skill, or if none. + /// /// - /// Returns scripts discovered via reflection by scanning for - /// methods annotated with . This discovery is - /// compatible with Native AOT because is annotated with + /// 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 override IReadOnlyList? Scripts + public virtual IReadOnlyList? Scripts => this._scripts.Value; + + /// + public sealed override bool HasResources => this.Resources is { Count: > 0 }; + + /// + public sealed override bool HasScripts => this.Scripts is { Count: > 0 }; + + /// + public sealed override ValueTask GetResourceAsync(string name, CancellationToken cancellationToken = default) { - get - { - if (!this._scriptsDiscovered) - { - this._reflectedScripts = this.DiscoverScripts(); - this._scriptsDiscovered = true; - } + var resource = this.Resources?.FirstOrDefault(r => r.Name == name); + return new(resource); + } - return this._reflectedScripts; - } + /// + public sealed 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/AgentInlineSkill.cs b/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentInlineSkill.cs index cdfb14a584..778fb11710 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,30 @@ public AgentInlineSkill( 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 bool HasResources => this._resources is { Count: > 0 }; + + /// + public override bool HasScripts => this._scripts is { Count: > 0 }; /// - 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..eaa9b56b1c 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 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..9c44408c7b 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 the method 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..5bced3785a 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,18 @@ public void MinimalClassSkill_HasNullOverrides_AndSynthesizesContent() // Act & Assert — null overrides Assert.Equal("minimal", skill.Frontmatter.Name); Assert.Null(skill.Resources); - Assert.Null(skill.Scripts); + Assert.False(skill.HasScripts); // 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,15 +50,15 @@ public void FullClassSkill_ReturnsOverriddenLists_AndCachesContent() 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] - public void ResourcesAndScripts_CanBeLazyLoaded_AndCached() + public async Task ResourcesAndScripts_CanBeLazyLoaded_AndCachedAsync() { // Arrange var skill = new LazyLoadedSkill(); @@ -104,7 +104,7 @@ public void AgentClassSkill_InvalidFrontmatter_ThrowsArgumentException() } [Fact] - public void PartialOverrides_OneCollectionNull_OtherHasValues() + public async Task PartialOverrides_OneCollectionNull_OtherHasValuesAsync() { // Arrange var resourceOnly = new ResourceOnlySkill(); @@ -117,6 +117,156 @@ public void PartialOverrides_OneCollectionNull_OtherHasValues() Assert.Single(scriptOnly.Scripts!); } + [Fact] + public void HasResources_ReturnsTrue_WhenSkillHasResources() + { + // Arrange + var skill = new FullClassSkill(); + + // Act & Assert + Assert.True(skill.HasResources); + } + + [Fact] + public void HasResources_ReturnsFalse_WhenSkillHasNoResources() + { + // Arrange + var skill = new MinimalClassSkill(); + + // Act & Assert + Assert.False(skill.HasResources); + } + + [Fact] + public void HasScripts_ReturnsTrue_WhenSkillHasScripts() + { + // Arrange + var skill = new FullClassSkill(); + + // Act & Assert + Assert.True(skill.HasScripts); + } + + [Fact] + public void HasScripts_ReturnsFalse_WhenSkillHasNoScripts() + { + // Arrange + var skill = new MinimalClassSkill(); + + // Act & Assert + Assert.False(skill.HasScripts); + } + + [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() { @@ -146,22 +296,19 @@ public async Task CreateScriptAndResource_WithSerializerOptions_HandleCustomType } [Fact] - public void Scripts_DiscoveredViaAttribute_WithCorrectNamesAndDescriptions() + public async Task Scripts_DiscoveredViaAttribute_WithCorrectNamesAndDescriptionsAsync() { // Arrange var skill = new AttributedScriptsSkill(); - // Act - var scripts = skill.Scripts; - - // 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"); + // 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"); - 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 +419,16 @@ public async Task Resources_DiscoveredViaAttribute_OnMethods_CanBeReadAsync() } [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,37 +446,37 @@ public void NoAttributedMembers_NoOverrides_ReturnsNull() // 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); - - Assert.NotNull(resourcesDiscoveredField); - Assert.NotNull(scriptsDiscoveredField); - Assert.NotNull(reflectedResourcesField); - Assert.NotNull(reflectedScriptsField); - Assert.False((bool)resourcesDiscoveredField!.GetValue(skill)!); - Assert.False((bool)scriptsDiscoveredField!.GetValue(skill)!); + var resourcesField = baseType.GetField("_resources", BindingFlags.Instance | BindingFlags.NonPublic); + var scriptsField = baseType.GetField("_scripts", BindingFlags.Instance | BindingFlags.NonPublic); + + 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] - public void SubclassOverride_TakesPrecedence_OverAttributes() + public async Task SubclassOverride_TakesPrecedence_OverAttributesAsync() { // Arrange — skill has attributes AND overrides Resources/Scripts var skill = new AttributedWithOverrideSkill(); @@ -382,7 +529,7 @@ public async Task SerializerOptions_UsedForReflectedMembersAsync() 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 +545,13 @@ public async Task SerializerOptions_UsedForReflectedMembersAsync() } [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..9e401688b9 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillScriptTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillScriptTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft. All rights reserved. +// Copyright (c) Microsoft. All rights reserved. using System; using System.Text.Json; @@ -105,7 +105,7 @@ public void ParametersSchema_ReturnsExpectedArraySchema() } [Fact] - public void Content_WithScripts_AppendsPerScriptEntries() + public async Task Content_WithScripts_AppendsPerScriptEntries() { // Arrange static Task RunnerAsync(AgentFileSkill s, AgentFileSkillScript sc, JsonElement? a, IServiceProvider? sp, CancellationToken ct) => Task.FromResult(null); @@ -118,7 +118,7 @@ public void Content_WithScripts_AppendsPerScriptEntries() 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 void Content_WithScripts_AppendsPerScriptEntries() } [Fact] - public void Content_WithoutScripts_ReturnsOriginalContent() + public async Task Content_WithoutScripts_ReturnsOriginalContentAsync() { // Arrange var fileSkill = new AgentFileSkill( @@ -139,14 +139,14 @@ public void Content_WithoutScripts_ReturnsOriginalContent() "/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 void Content_WithScripts_IsCached() 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 @@ await Assert.ThrowsAsync( } [Fact] - public void Content_WithScripts_ContainsDefaultParametersSchema() + public async Task Content_WithScripts_ContainsDefaultParametersSchema() { // Arrange static Task RunnerAsync(AgentFileSkill s, AgentFileSkillScript sc, JsonElement? a, IServiceProvider? sp, CancellationToken ct) => Task.FromResult(null); @@ -244,7 +244,7 @@ public void Content_WithScripts_ContainsDefaultParametersSchema() 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..0f4c5c0202 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillsSourceScriptTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillsSourceScriptTests.cs @@ -46,9 +46,10 @@ public async Task GetSkillsAsync_WithScriptFiles_DiscoversScriptsAsync() // 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); + Assert.True(skill.HasScripts); + var script = await skill.GetScriptAsync("scripts/convert.py"); + Assert.NotNull(script); + Assert.Equal("scripts/convert.py", script!.Name); } [Fact] @@ -69,14 +70,14 @@ public async Task GetSkillsAsync_WithMultipleScriptExtensions_DiscoversAllAsync( // 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 + Assert.True(skills[0].HasScripts); + 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 +95,7 @@ public async Task GetSkillsAsync_NonScriptExtensionsAreNotDiscoveredAsync() // Assert Assert.Single(skills); - Assert.Empty(skills[0].Scripts!); + Assert.False(skills[0].HasScripts); } [Fact] @@ -109,8 +110,7 @@ public async Task GetSkillsAsync_NoScriptFiles_ReturnsEmptyScriptsAsync() // Assert Assert.Single(skills); - Assert.NotNull(skills[0].Scripts); - Assert.Empty(skills[0].Scripts!); + Assert.False(skills[0].HasScripts); } [Fact] @@ -128,7 +128,7 @@ public async Task GetSkillsAsync_ScriptsOutsideScriptsDir_AreNotDiscoveredAsync( // Assert — neither file is in the default scripts/ directory, so no scripts are discovered Assert.Single(skills); - Assert.Empty(skills[0].Scripts!); + Assert.False(skills[0].HasScripts); } [Fact] @@ -150,7 +150,7 @@ public async Task GetSkillsAsync_WithRunner_ScriptsCanRunAsync() // 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 +175,7 @@ public async Task GetSkillsAsync_ScriptsWithNoRunner_ThrowsOnRunAsync() // 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 +195,10 @@ public async Task GetSkillsAsync_CustomScriptExtensions_OnlyDiscoversMatchingAsy // Assert Assert.Single(skills); - Assert.Single(skills[0].Scripts!); - Assert.Equal("scripts/run.rb", skills[0].Scripts![0].Name); + Assert.True(skills[0].HasScripts); + var rbScript = await skills[0].GetScriptAsync("scripts/run.rb"); + Assert.NotNull(rbScript); + Assert.Equal("scripts/run.rb", rbScript!.Name); } [Fact] @@ -217,7 +219,7 @@ public async Task GetSkillsAsync_ExecutorReceivesArgumentsAsync() 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 +242,10 @@ public async Task GetSkillsAsync_ScriptDirectoriesWithNestedPath_DiscoversScript // 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); + Assert.True(skills[0].HasScripts); + 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 +271,13 @@ public async Task GetSkillsAsync_ScriptDirectoryWithDotSlashPrefix_DiscoversScri // Assert — scripts are discovered with names identical to using directories without "./" Assert.Single(skills); - Assert.Equal(directories.Length, skills[0].Scripts!.Count); + Assert.True(skills[0].HasScripts); 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..86c486cced 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentInlineSkillTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentInlineSkillTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft. All rights reserved. +// Copyright (c) Microsoft. All rights reserved. using System; using System.Text.Json; @@ -105,13 +105,13 @@ public void Constructor_WithAllProps_NullInstructions_Throws() } [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 void Content_ContainsNameDescriptionAndInstructions() } [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 void Content_EscapesXmlCharacters() } [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 void Content_IncludesResourcesAddedBeforeFirstAccess() } [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 void Content_IncludesDelegateResourcesAddedBeforeFirstAccess() } [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 void Content_IncludesScriptsAddedBeforeFirstAccess() } [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 void Content_IncludesResourcesAndScriptsAddedBeforeFirstAccess() skill.AddScript("s1", () => "ok"); // Act - var content = skill.Content; + var content = await skill.GetContentAsync(); // Assert Assert.Contains("", content); @@ -227,14 +227,14 @@ public void Content_IncludesResourcesAndScriptsAddedBeforeFirstAccess() } [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,7 +280,7 @@ public void Resources_WhenNoneAdded_ReturnsNull() var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); // Act & Assert - Assert.Null(skill.Resources); + Assert.Null(skill.GetTestResources()); } [Fact] @@ -290,7 +290,125 @@ public void Scripts_WhenNoneAdded_ReturnsNull() var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); // Act & Assert - Assert.Null(skill.Scripts); + Assert.False(skill.HasScripts); + } + + [Fact] + public void HasResources_ReturnsTrue_WhenResourcesAdded() + { + // Arrange + var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); + skill.AddResource("r1", "v1"); + + // Act & Assert + Assert.True(skill.HasResources); + } + + [Fact] + public void HasResources_ReturnsFalse_WhenNoResourcesAdded() + { + // Arrange + var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); + + // Act & Assert + Assert.False(skill.HasResources); + } + + [Fact] + public void HasScripts_ReturnsTrue_WhenScriptsAdded() + { + // Arrange + var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); + skill.AddScript("s1", () => "ok"); + + // Act & Assert + Assert.True(skill.HasScripts); + } + + [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 +451,13 @@ public void AddScript_ReturnsSameInstance_ForChaining() } [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 +465,58 @@ public void Content_NoResourcesOrScripts_DoesNotContainResourcesOrScriptsTags() } [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 +524,7 @@ public void Content_ScriptWithoutParametersOrDescription_UsesSelfClosingTag() } [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 +532,7 @@ public void Content_ResourceWithDescription_IncludesDescriptionAttribute() 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 +555,7 @@ public async Task AddScript_SkillLevelSerializerOptions_AppliedToScriptAsync() 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 +579,7 @@ public async Task AddScript_PerScriptSerializerOptions_OverridesSkillLevelAsync( 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 +595,7 @@ public async Task AddResource_SkillLevelSerializerOptions_AppliedToDelegateResou 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 +612,7 @@ public async Task AddResource_PerResourceSerializerOptions_OverridesSkillLevelAs 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..ae7cce790a --- /dev/null +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillTestExtensions.cs @@ -0,0 +1,57 @@ +// 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 +/// and . +/// These helpers exist purely to allow unit tests to inspect the concrete enumerated list a +/// skill carries; they intentionally use reflection so that no test-only members leak into +/// production types. +/// +internal static class AgentSkillTestExtensions +{ + public static IReadOnlyList? GetTestResources(this AgentSkill skill) + { + // AgentClassSkill: "_reflectedResources" private field (lazy reflection-discovered). + // Trigger discovery by calling HasResources first, then read the field. + for (var type = skill.GetType(); type is not null; type = type.BaseType) + { + var field = type.GetField("_reflectedResources", BindingFlags.NonPublic | BindingFlags.Instance); + if (field is not null) + { + // Trigger lazy discovery + _ = skill.HasResources; + return UnwrapList(field.GetValue(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..0ea6b29712 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillsProviderTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillsProviderTests.cs @@ -416,7 +416,7 @@ public async Task Builder_UseFileSkillWithOptionsResourceFilter_FiltersResources // 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 +445,279 @@ public async Task LoadSkill_DefaultOptions_ReturnsFullContentAsync() 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 +1271,5 @@ public TestClassSkill(string name, string description, string instructions) 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..ea35f74236 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 async Task GetSkillsAsync_FilesWithMatchingExtensions_DiscoveredAsResourc // 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 async Task GetSkillsAsync_FilesWithNonMatchingExtensions_NotDiscoveredAsy // 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 async Task GetSkillsAsync_SkillMdFile_NotIncludedAsResourceAsync() // 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 async Task GetSkillsAsync_NestedResourceFiles_DiscoveredAsync() // 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 async Task GetSkillsAsync_CustomResourceExtensions_UsedForDiscoveryAsync( // 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 async Task Constructor_NullExtensions_UsesDefaultsAsync() // 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 async Task GetSkillsAsync_ResourceInSkillRoot_NotDiscoveredByDefaultAsync // 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 async Task GetSkillsAsync_ResourceInSkillRoot_DiscoveredWhenRootDirectory // 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 async Task GetSkillsAsync_ResourceInNonSpecDirectory_NotDiscoveredByDefau // 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 async Task GetSkillsAsync_CustomResourceDirectories_ReplacesDefaultsAsync // 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 async Task GetSkillsAsync_NoResourceFiles_ReturnsEmptyResourcesAsync() // Assert Assert.Single(skills); - Assert.Empty(skills[0].Resources!); + Assert.Empty(skills[0].GetTestResources()!); } [Fact] @@ -588,7 +588,7 @@ public async Task ReadSkillResourceAsync_ValidResource_ReturnsContentAsync() 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 async Task GetSkillsAsync_SymlinkInPath_SkipsSymlinkedResourcesAsync() // 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 async Task GetSkillsAsync_SymlinkedResourceDirectory_SkipsWithoutEnumerat // 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 async Task GetSkillsAsync_SymlinkedScriptDirectory_SkipsWithoutEnumeratin // 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.False(skill.HasScripts); } [Fact] @@ -791,7 +791,7 @@ public async Task GetSkillsAsync_SymlinkedIntermediateSegment_SkipsCustomDirecto // 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 async Task GetSkillsAsync_DuplicateDirectoriesAfterNormalization_NoDuplic // 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 async Task GetSkillsAsync_TrailingSlashDirectoryNormalized_NoDuplicateRes // 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,10 @@ public async Task GetSkillsAsync_BackslashDirectoryNormalized_NoDuplicateScripts // Assert — backslash variant deduplicated Assert.Single(skills); - Assert.Single(skills[0].Scripts!); - Assert.Equal("scripts/run.py", skills[0].Scripts![0].Name); + Assert.True(skills[0].HasScripts); + var script = await skills[0].GetScriptAsync("scripts/run.py"); + Assert.NotNull(script); + Assert.Equal("scripts/run.py", script!.Name); } [Theory] @@ -1093,8 +1095,8 @@ public async Task GetSkillsAsync_ResourceDirectoryWithDotSlashPrefix_DiscoversRe // 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 +1119,8 @@ public async Task GetSkillsAsync_ResourceDirectoriesWithNestedPath_DiscoversReso // 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 +1190,10 @@ public async Task GetSkillsAsync_ScriptInSkillRoot_DiscoveredWhenRootDirectoryCo // 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); + Assert.True(skill.HasScripts); + var script = await skill.GetScriptAsync("run.py"); + Assert.NotNull(script); + Assert.Equal("run.py", script!.Name); } #if NET @@ -1229,8 +1233,8 @@ public async Task GetSkillsAsync_SymlinkedFileInRealDirectory_SkipsSymlinkedFile // 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/TestSkillTypes.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/TestSkillTypes.cs index a2fb4155e4..523b5300e4 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/TestSkillTypes.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/TestSkillTypes.cs @@ -31,13 +31,10 @@ public TestAgentSkill(string name, string description, string content) public override AgentSkillFrontmatter Frontmatter => this._frontmatter; /// - public override string Content => this._content; + public override ValueTask GetContentAsync(CancellationToken cancellationToken = default) => new(this._content); /// - public override IReadOnlyList? Resources => null; - - /// - public override IReadOnlyList? Scripts => null; + public override bool HasScripts => false; } /// From d26ec2cda4fa88182095582ff0768633488daa7b Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Fri, 22 May 2026 14:45:58 +0100 Subject: [PATCH 2/7] 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> --- .../Skills/AgentSkillsProvider.cs | 12 ++++++------ .../AgentSkillResourceAttribute.cs | 2 +- .../Programmatic/AgentSkillScriptAttribute.cs | 2 +- .../AgentSkills/AgentClassSkillTests.cs | 8 ++++---- .../AgentSkills/AgentFileSkillScriptTests.cs | 4 ++-- .../AgentSkills/AgentSkillTestExtensions.cs | 18 ++---------------- 6 files changed, 16 insertions(+), 30 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProvider.cs b/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProvider.cs index 048ba36416..b23c4b39cd 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProvider.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProvider.cs @@ -359,14 +359,14 @@ private async Task LoadSkillAsync(IList skills, string skill return $"Error: Skill '{skillName}' not found."; } - var script = await skill.GetScriptAsync(scriptName, cancellationToken).ConfigureAwait(false); - 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/Programmatic/AgentSkillResourceAttribute.cs b/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentSkillResourceAttribute.cs index eaa9b56b1c..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 and 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 9c44408c7b..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 method 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 5bced3785a..17c42af46d 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentClassSkillTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentClassSkillTests.cs @@ -58,7 +58,7 @@ public async Task FullClassSkill_ReturnsOverriddenLists_AndCachesContentAsync() } [Fact] - public async Task ResourcesAndScripts_CanBeLazyLoaded_AndCachedAsync() + public void ResourcesAndScripts_CanBeLazyLoaded_AndCached() { // Arrange var skill = new LazyLoadedSkill(); @@ -104,7 +104,7 @@ public void AgentClassSkill_InvalidFrontmatter_ThrowsArgumentException() } [Fact] - public async Task PartialOverrides_OneCollectionNull_OtherHasValuesAsync() + public void PartialOverrides_OneCollectionNull_OtherHasValues() { // Arrange var resourceOnly = new ResourceOnlySkill(); @@ -296,7 +296,7 @@ public async Task CreateScriptAndResource_WithSerializerOptions_HandleCustomType } [Fact] - public async Task Scripts_DiscoveredViaAttribute_WithCorrectNamesAndDescriptionsAsync() + public void Scripts_DiscoveredViaAttribute_WithCorrectNamesAndDescriptions() { // Arrange var skill = new AttributedScriptsSkill(); @@ -476,7 +476,7 @@ public void NoAttributedMembers_NoOverrides_ReturnsNull() } [Fact] - public async Task SubclassOverride_TakesPrecedence_OverAttributesAsync() + public void SubclassOverride_TakesPrecedence_OverAttributes() { // Arrange — skill has attributes AND overrides Resources/Scripts var skill = new AttributedWithOverrideSkill(); diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillScriptTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillScriptTests.cs index 9e401688b9..f04195bc8f 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 void ParametersSchema_ReturnsExpectedArraySchema() } [Fact] - public async Task 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); @@ -232,7 +232,7 @@ await Assert.ThrowsAsync( } [Fact] - public async Task 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); diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillTestExtensions.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillTestExtensions.cs index ae7cce790a..732aa27d37 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillTestExtensions.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillTestExtensions.cs @@ -12,27 +12,13 @@ namespace Microsoft.Agents.AI.UnitTests.AgentSkills; /// /// The public API exposes resources only through /// and . -/// These helpers exist purely to allow unit tests to inspect the concrete enumerated list a -/// skill carries; they intentionally use reflection so that no test-only members leak into -/// production types. +/// 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) { - // AgentClassSkill: "_reflectedResources" private field (lazy reflection-discovered). - // Trigger discovery by calling HasResources first, then read the field. - for (var type = skill.GetType(); type is not null; type = type.BaseType) - { - var field = type.GetField("_reflectedResources", BindingFlags.NonPublic | BindingFlags.Instance); - if (field is not null) - { - // Trigger lazy discovery - _ = skill.HasResources; - return UnwrapList(field.GetValue(skill)); - } - } - // AgentFileSkill / AgentInlineSkill: private "_resources" field. for (var type = skill.GetType(); type is not null; type = type.BaseType) { From 85d1a1c8dfb88b9322134ab91f5e2acd5d347ad9 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Fri, 22 May 2026 15:16:10 +0100 Subject: [PATCH 3/7] Fix formatting: add UTF-8 BOM and remove unused using Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../AgentSkills/AgentFileSkillScriptTests.cs | 2 +- .../AgentSkills/AgentFileSkillsSourceScriptTests.cs | 1 - .../AgentSkills/AgentInlineSkillTests.cs | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillScriptTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillScriptTests.cs index f04195bc8f..9a27528051 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillScriptTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillScriptTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft. All rights reserved. +// Copyright (c) Microsoft. All rights reserved. using System; using System.Text.Json; diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillsSourceScriptTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillsSourceScriptTests.cs index 0f4c5c0202..6815ea78b0 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; diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentInlineSkillTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentInlineSkillTests.cs index 86c486cced..c59e021e59 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentInlineSkillTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentInlineSkillTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft. All rights reserved. +// Copyright (c) Microsoft. All rights reserved. using System; using System.Text.Json; From 474bbf8cb61d0c9b89546d0efb5fb6e8c4a8ec25 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Fri, 22 May 2026 16:29:34 +0100 Subject: [PATCH 4/7] Fix XML cref: Resources/Scripts are on AgentClassSkill Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../AgentSkills/Agent_Step03_ClassBasedSkills/Program.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 @@ /// 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 { From be30fbb24b93a36114391a33168a68c7f05542a2 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Mon, 25 May 2026 17:10:54 +0100 Subject: [PATCH 5/7] 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> --- .../Microsoft.Agents.AI/Skills/AgentSkill.cs | 22 +--------- .../Skills/AgentSkillsProvider.cs | 41 ++++++------------- .../Skills/File/AgentFileSkill.cs | 6 --- .../Skills/Programmatic/AgentClassSkill.cs | 6 --- .../Skills/Programmatic/AgentInlineSkill.cs | 6 --- .../AgentSkills/AgentClassSkillTests.cs | 41 ------------------- .../AgentFileSkillsSourceScriptTests.cs | 11 ++--- .../AgentSkills/AgentInlineSkillTests.cs | 36 +--------------- .../AgentSkills/AgentSkillTestExtensions.cs | 2 +- .../AgentSkills/AgentSkillsProviderTests.cs | 13 +++--- .../AgentSkills/FileAgentSkillLoaderTests.cs | 4 +- .../AgentSkills/TestSkillTypes.cs | 3 -- 12 files changed, 30 insertions(+), 161 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkill.cs b/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkill.cs index 48a6142d54..38342d6816 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkill.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkill.cs @@ -44,24 +44,6 @@ public abstract class AgentSkill /// public abstract ValueTask GetContentAsync(CancellationToken cancellationToken = default); - /// - /// Gets a value indicating whether this skill may have resources that can be served via . - /// - /// - /// The default implementation returns . For skills whose resource - /// availability cannot be determined in advance (e.g. MCP skills), this should return - /// as a conservative default. - /// - public virtual bool HasResources => false; - - /// - /// Gets a value indicating whether this skill may have scripts that can be served via . - /// - /// - /// The default implementation returns . - /// - public virtual bool HasScripts => false; - /// /// Gets a resource owned by this skill by name. /// @@ -72,7 +54,7 @@ public abstract class AgentSkill /// /// /// The default implementation returns . Override in derived classes that - /// expose resources, and set to . + /// expose resources. /// public virtual ValueTask GetResourceAsync( string name, @@ -88,7 +70,7 @@ public abstract class AgentSkill /// /// /// The default implementation returns . Override in derived classes that - /// expose scripts, and set to . + /// expose scripts. /// public virtual ValueTask GetScriptAsync( string name, diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProvider.cs b/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProvider.cs index b23c4b39cd..4d9ae154f0 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProvider.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProvider.cs @@ -186,13 +186,10 @@ private async Task CreateContextAsync(InvokingContext context, Cancel return await base.ProvideAIContextAsync(context, cancellationToken).ConfigureAwait(false); } - bool hasScripts = skills.Any(s => s.HasScripts); - bool hasResources = skills.Any(s => s.HasResources); - 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,7 +216,7 @@ private async Task GetOrCreateContextAsync(InvokingContext context, C } } - private IList BuildTools(IList skills, bool hasScripts, bool hasResources) + private IList BuildTools(IList skills) { IList tools = [ @@ -227,21 +224,12 @@ private IList BuildTools(IList skills, bool hasScripts, (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 @@ private IList BuildTools(IList skills, bool hasScripts, return [.. tools, scriptFunction]; } - private string? BuildSkillsInstructions(IList skills, bool includeScriptInstructions, bool includeResourceInstructions) + private string? BuildSkillsInstructions(IList skills) { string promptTemplate = this._options?.SkillsInstructionPrompt ?? DefaultSkillsInstructionPrompt; @@ -270,21 +258,18 @@ private IList BuildTools(IList skills, bool hasScripts, 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(); } diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkill.cs b/dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkill.cs index e0712cfe25..d1c792de21 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkill.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkill.cs @@ -66,12 +66,6 @@ public override ValueTask GetContentAsync(CancellationToken cancellation /// public string Path { get; } - /// - public override bool HasResources => this._resources.Count > 0; - - /// - public override bool HasScripts => this._scripts.Count > 0; - /// public override ValueTask GetResourceAsync(string name, CancellationToken cancellationToken = default) { diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentClassSkill.cs b/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentClassSkill.cs index 1d706b6f4e..32f461e32a 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentClassSkill.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentClassSkill.cs @@ -167,12 +167,6 @@ protected AgentClassSkill() /// public virtual IReadOnlyList? Scripts => this._scripts.Value; - /// - public sealed override bool HasResources => this.Resources is { Count: > 0 }; - - /// - public sealed override bool HasScripts => this.Scripts is { Count: > 0 }; - /// public sealed override ValueTask GetResourceAsync(string name, CancellationToken cancellationToken = default) { diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentInlineSkill.cs b/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentInlineSkill.cs index 778fb11710..4fb2b045cb 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentInlineSkill.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentInlineSkill.cs @@ -98,12 +98,6 @@ public override ValueTask GetContentAsync(CancellationToken cancellation return new(this._cachedContent ??= AgentInlineSkillContentBuilder.Build(this.Frontmatter.Name, this.Frontmatter.Description, this._instructions, this._resources, this._scripts)); } - /// - public override bool HasResources => this._resources is { Count: > 0 }; - - /// - public override bool HasScripts => this._scripts is { Count: > 0 }; - /// public override ValueTask GetResourceAsync(string name, CancellationToken cancellationToken = default) { diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentClassSkillTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentClassSkillTests.cs index 17c42af46d..1248866a52 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentClassSkillTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentClassSkillTests.cs @@ -26,7 +26,6 @@ public async Task MinimalClassSkill_HasNullOverrides_AndSynthesizesContentAsync( // Act & Assert — null overrides Assert.Equal("minimal", skill.Frontmatter.Name); Assert.Null(skill.Resources); - Assert.False(skill.HasScripts); // Act & Assert — synthesized XML content Assert.Contains("minimal", await skill.GetContentAsync()); @@ -117,46 +116,6 @@ public void PartialOverrides_OneCollectionNull_OtherHasValues() Assert.Single(scriptOnly.Scripts!); } - [Fact] - public void HasResources_ReturnsTrue_WhenSkillHasResources() - { - // Arrange - var skill = new FullClassSkill(); - - // Act & Assert - Assert.True(skill.HasResources); - } - - [Fact] - public void HasResources_ReturnsFalse_WhenSkillHasNoResources() - { - // Arrange - var skill = new MinimalClassSkill(); - - // Act & Assert - Assert.False(skill.HasResources); - } - - [Fact] - public void HasScripts_ReturnsTrue_WhenSkillHasScripts() - { - // Arrange - var skill = new FullClassSkill(); - - // Act & Assert - Assert.True(skill.HasScripts); - } - - [Fact] - public void HasScripts_ReturnsFalse_WhenSkillHasNoScripts() - { - // Arrange - var skill = new MinimalClassSkill(); - - // Act & Assert - Assert.False(skill.HasScripts); - } - [Fact] public async Task GetResourceAsync_ExistingName_ReturnsResourceAsync() { diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillsSourceScriptTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillsSourceScriptTests.cs index 6815ea78b0..32e018ae90 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillsSourceScriptTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillsSourceScriptTests.cs @@ -45,7 +45,6 @@ public async Task GetSkillsAsync_WithScriptFiles_DiscoversScriptsAsync() // Assert Assert.Single(skills); var skill = skills[0]; - Assert.True(skill.HasScripts); var script = await skill.GetScriptAsync("scripts/convert.py"); Assert.NotNull(script); Assert.Equal("scripts/convert.py", script!.Name); @@ -70,7 +69,6 @@ public async Task GetSkillsAsync_WithMultipleScriptExtensions_DiscoversAllAsync( // Assert Assert.Single(skills); // Assert — verify all expected scripts are discoverable - Assert.True(skills[0].HasScripts); 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); @@ -94,7 +92,7 @@ public async Task GetSkillsAsync_NonScriptExtensionsAreNotDiscoveredAsync() // Assert Assert.Single(skills); - Assert.False(skills[0].HasScripts); + Assert.Null(await skills[0].GetScriptAsync("scripts/data.txt")); } [Fact] @@ -109,7 +107,7 @@ public async Task GetSkillsAsync_NoScriptFiles_ReturnsEmptyScriptsAsync() // Assert Assert.Single(skills); - Assert.False(skills[0].HasScripts); + Assert.Null(await skills[0].GetScriptAsync("any-script")); } [Fact] @@ -127,7 +125,7 @@ public async Task GetSkillsAsync_ScriptsOutsideScriptsDir_AreNotDiscoveredAsync( // Assert — neither file is in the default scripts/ directory, so no scripts are discovered Assert.Single(skills); - Assert.False(skills[0].HasScripts); + Assert.Null(await skills[0].GetScriptAsync("convert.py")); } [Fact] @@ -194,7 +192,6 @@ public async Task GetSkillsAsync_CustomScriptExtensions_OnlyDiscoversMatchingAsy // Assert Assert.Single(skills); - Assert.True(skills[0].HasScripts); var rbScript = await skills[0].GetScriptAsync("scripts/run.rb"); Assert.NotNull(rbScript); Assert.Equal("scripts/run.rb", rbScript!.Name); @@ -241,7 +238,6 @@ public async Task GetSkillsAsync_ScriptDirectoriesWithNestedPath_DiscoversScript // Assert — script file inside the deeply nested directory is discovered Assert.Single(skills); - Assert.True(skills[0].HasScripts); var nestedScript = await skills[0].GetScriptAsync("f1/f2/f3/run.py"); Assert.NotNull(nestedScript); Assert.Equal("f1/f2/f3/run.py", nestedScript!.Name); @@ -270,7 +266,6 @@ public async Task GetSkillsAsync_ScriptDirectoryWithDotSlashPrefix_DiscoversScri // Assert — scripts are discovered with names identical to using directories without "./" Assert.Single(skills); - Assert.True(skills[0].HasScripts); foreach (string directory in directories) { string expectedName = $"{directory.Substring(2)}/run.py"; diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentInlineSkillTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentInlineSkillTests.cs index c59e021e59..c6ae2212bf 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentInlineSkillTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentInlineSkillTests.cs @@ -284,45 +284,13 @@ public void Resources_WhenNoneAdded_ReturnsNull() } [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.False(skill.HasScripts); - } - - [Fact] - public void HasResources_ReturnsTrue_WhenResourcesAdded() - { - // Arrange - var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); - skill.AddResource("r1", "v1"); - - // Act & Assert - Assert.True(skill.HasResources); - } - - [Fact] - public void HasResources_ReturnsFalse_WhenNoResourcesAdded() - { - // Arrange - var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); - - // Act & Assert - Assert.False(skill.HasResources); - } - - [Fact] - public void HasScripts_ReturnsTrue_WhenScriptsAdded() - { - // Arrange - var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); - skill.AddScript("s1", () => "ok"); - - // Act & Assert - Assert.True(skill.HasScripts); + Assert.Null(await skill.GetScriptAsync("nonexistent")); } [Fact] diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillTestExtensions.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillTestExtensions.cs index 732aa27d37..5a67bcf62c 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillTestExtensions.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillTestExtensions.cs @@ -11,7 +11,7 @@ namespace Microsoft.Agents.AI.UnitTests.AgentSkills; /// /// /// The public API exposes resources only through -/// and . +/// . /// These helpers exist purely to allow unit tests for and /// to inspect the concrete enumerated list a skill carries. /// diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillsProviderTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillsProviderTests.cs index 0ea6b29712..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 async Task InvokingCoreAsync_WithSkills_AppendsInstructionsAndToolsAsync( 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 async Task InvokingCoreAsync_WithScripts_IncludesRunSkillScriptToolAsync( } [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 async Task InvokingCoreAsync_WithoutScripts_NoRunSkillScriptToolAsync() // 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] diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs index ea35f74236..cd362ce64e 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs @@ -751,7 +751,7 @@ public async Task GetSkillsAsync_SymlinkedScriptDirectory_SkipsWithoutEnumeratin // 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.False(skill.HasScripts); + Assert.Null(await skill.GetScriptAsync("any-script")); } [Fact] @@ -1066,7 +1066,6 @@ public async Task GetSkillsAsync_BackslashDirectoryNormalized_NoDuplicateScripts // Assert — backslash variant deduplicated Assert.Single(skills); - Assert.True(skills[0].HasScripts); var script = await skills[0].GetScriptAsync("scripts/run.py"); Assert.NotNull(script); Assert.Equal("scripts/run.py", script!.Name); @@ -1190,7 +1189,6 @@ public async Task GetSkillsAsync_ScriptInSkillRoot_DiscoveredWhenRootDirectoryCo // Assert — script at the skill root should be discovered var skill = skills.FirstOrDefault(s => s.Frontmatter.Name == "root-script-skill"); Assert.NotNull(skill); - Assert.True(skill.HasScripts); var script = await skill.GetScriptAsync("run.py"); Assert.NotNull(script); Assert.Equal("run.py", script!.Name); diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/TestSkillTypes.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/TestSkillTypes.cs index 523b5300e4..7110fb438b 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/TestSkillTypes.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/TestSkillTypes.cs @@ -32,9 +32,6 @@ public TestAgentSkill(string name, string description, string content) /// public override ValueTask GetContentAsync(CancellationToken cancellationToken = default) => new(this._content); - - /// - public override bool HasScripts => false; } /// From e60a8c4c1421d74f555793897e6977b224c6ae1e Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Mon, 25 May 2026 17:11:52 +0100 Subject: [PATCH 6/7] Add blank line for readability in file-based skills sample Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../AgentSkills/Agent_Step01_FileBasedSkills/Program.cs | 1 + 1 file changed, 1 insertion(+) 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 @@ var skillsProvider = new AgentSkillsProvider( Path.Combine(AppContext.BaseDirectory, "skills"), SubprocessScriptRunner.RunAsync); + // --- Agent Setup --- AIAgent agent = new AzureOpenAIClient(new Uri(endpoint), new DefaultAzureCredential()) .GetResponsesClient() From 89550d81898b58c25f01c8ab3a018d247862d9af Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Mon, 25 May 2026 17:44:20 +0100 Subject: [PATCH 7/7] 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> --- .../AgentSkills/HostedAgentSkillsPatternTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 async Task AgentSkillsProvider_WithDownloadedSkills_AdvertisesAndLoadsAsy 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]