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() 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 { diff --git a/dotnet/src/Microsoft.Agents.AI/CompatibilitySuppressions.xml b/dotnet/src/Microsoft.Agents.AI/CompatibilitySuppressions.xml index 6a2c790f22..a7f87560bf 100644 --- a/dotnet/src/Microsoft.Agents.AI/CompatibilitySuppressions.xml +++ b/dotnet/src/Microsoft.Agents.AI/CompatibilitySuppressions.xml @@ -43,6 +43,27 @@ lib/net10.0/Microsoft.Agents.AI.dll true + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Content + lib/net10.0/Microsoft.Agents.AI.dll + lib/net10.0/Microsoft.Agents.AI.dll + true + + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Resources + lib/net10.0/Microsoft.Agents.AI.dll + lib/net10.0/Microsoft.Agents.AI.dll + true + + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Scripts + lib/net10.0/Microsoft.Agents.AI.dll + lib/net10.0/Microsoft.Agents.AI.dll + true + CP0002 M:Microsoft.Agents.AI.AgentSkillScript.RunAsync(Microsoft.Agents.AI.AgentSkill,Microsoft.Extensions.AI.AIFunctionArguments,System.Threading.CancellationToken) @@ -106,6 +127,27 @@ lib/net472/Microsoft.Agents.AI.dll true + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Content + lib/net472/Microsoft.Agents.AI.dll + lib/net472/Microsoft.Agents.AI.dll + true + + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Resources + lib/net472/Microsoft.Agents.AI.dll + lib/net472/Microsoft.Agents.AI.dll + true + + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Scripts + lib/net472/Microsoft.Agents.AI.dll + lib/net472/Microsoft.Agents.AI.dll + true + CP0002 M:Microsoft.Agents.AI.AgentSkillScript.RunAsync(Microsoft.Agents.AI.AgentSkill,Microsoft.Extensions.AI.AIFunctionArguments,System.Threading.CancellationToken) @@ -169,6 +211,27 @@ lib/net8.0/Microsoft.Agents.AI.dll true + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Content + lib/net8.0/Microsoft.Agents.AI.dll + lib/net8.0/Microsoft.Agents.AI.dll + true + + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Resources + lib/net8.0/Microsoft.Agents.AI.dll + lib/net8.0/Microsoft.Agents.AI.dll + true + + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Scripts + lib/net8.0/Microsoft.Agents.AI.dll + lib/net8.0/Microsoft.Agents.AI.dll + true + CP0002 M:Microsoft.Agents.AI.AgentSkillScript.RunAsync(Microsoft.Agents.AI.AgentSkill,Microsoft.Extensions.AI.AIFunctionArguments,System.Threading.CancellationToken) @@ -232,6 +295,27 @@ lib/net9.0/Microsoft.Agents.AI.dll true + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Content + lib/net9.0/Microsoft.Agents.AI.dll + lib/net9.0/Microsoft.Agents.AI.dll + true + + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Resources + lib/net9.0/Microsoft.Agents.AI.dll + lib/net9.0/Microsoft.Agents.AI.dll + true + + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Scripts + lib/net9.0/Microsoft.Agents.AI.dll + lib/net9.0/Microsoft.Agents.AI.dll + true + CP0002 M:Microsoft.Agents.AI.AgentSkillScript.RunAsync(Microsoft.Agents.AI.AgentSkill,Microsoft.Extensions.AI.AIFunctionArguments,System.Threading.CancellationToken) @@ -295,6 +379,27 @@ lib/netstandard2.0/Microsoft.Agents.AI.dll true + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Content + lib/netstandard2.0/Microsoft.Agents.AI.dll + lib/netstandard2.0/Microsoft.Agents.AI.dll + true + + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Resources + lib/netstandard2.0/Microsoft.Agents.AI.dll + lib/netstandard2.0/Microsoft.Agents.AI.dll + true + + + CP0002 + M:Microsoft.Agents.AI.AgentSkill.get_Scripts + lib/netstandard2.0/Microsoft.Agents.AI.dll + lib/netstandard2.0/Microsoft.Agents.AI.dll + true + CP0002 M:Microsoft.Agents.AI.AgentSkillScript.RunAsync(Microsoft.Agents.AI.AgentSkill,Microsoft.Extensions.AI.AIFunctionArguments,System.Threading.CancellationToken) @@ -316,6 +421,13 @@ lib/netstandard2.0/Microsoft.Agents.AI.dll true + + CP0005 + M:Microsoft.Agents.AI.AgentSkill.GetContentAsync(System.Threading.CancellationToken) + lib/net10.0/Microsoft.Agents.AI.dll + lib/net10.0/Microsoft.Agents.AI.dll + true + CP0005 M:Microsoft.Agents.AI.AgentSkillScript.RunAsync(Microsoft.Agents.AI.AgentSkill,System.Nullable{System.Text.Json.JsonElement},System.IServiceProvider,System.Threading.CancellationToken) @@ -323,6 +435,13 @@ lib/net10.0/Microsoft.Agents.AI.dll true + + CP0005 + M:Microsoft.Agents.AI.AgentSkill.GetContentAsync(System.Threading.CancellationToken) + lib/net472/Microsoft.Agents.AI.dll + lib/net472/Microsoft.Agents.AI.dll + true + CP0005 M:Microsoft.Agents.AI.AgentSkillScript.RunAsync(Microsoft.Agents.AI.AgentSkill,System.Nullable{System.Text.Json.JsonElement},System.IServiceProvider,System.Threading.CancellationToken) @@ -330,6 +449,13 @@ lib/net472/Microsoft.Agents.AI.dll true + + CP0005 + M:Microsoft.Agents.AI.AgentSkill.GetContentAsync(System.Threading.CancellationToken) + lib/net8.0/Microsoft.Agents.AI.dll + lib/net8.0/Microsoft.Agents.AI.dll + true + CP0005 M:Microsoft.Agents.AI.AgentSkillScript.RunAsync(Microsoft.Agents.AI.AgentSkill,System.Nullable{System.Text.Json.JsonElement},System.IServiceProvider,System.Threading.CancellationToken) @@ -337,6 +463,13 @@ lib/net8.0/Microsoft.Agents.AI.dll true + + CP0005 + M:Microsoft.Agents.AI.AgentSkill.GetContentAsync(System.Threading.CancellationToken) + lib/net9.0/Microsoft.Agents.AI.dll + lib/net9.0/Microsoft.Agents.AI.dll + true + CP0005 M:Microsoft.Agents.AI.AgentSkillScript.RunAsync(Microsoft.Agents.AI.AgentSkill,System.Nullable{System.Text.Json.JsonElement},System.IServiceProvider,System.Threading.CancellationToken) @@ -344,6 +477,13 @@ lib/net9.0/Microsoft.Agents.AI.dll true + + CP0005 + M:Microsoft.Agents.AI.AgentSkill.GetContentAsync(System.Threading.CancellationToken) + lib/netstandard2.0/Microsoft.Agents.AI.dll + lib/netstandard2.0/Microsoft.Agents.AI.dll + true + CP0005 M:Microsoft.Agents.AI.AgentSkillScript.RunAsync(Microsoft.Agents.AI.AgentSkill,System.Nullable{System.Text.Json.JsonElement},System.IServiceProvider,System.Threading.CancellationToken) diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkill.cs b/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkill.cs index 6f549301d0..38342d6816 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkill.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkill.cs @@ -1,7 +1,8 @@ // Copyright (c) Microsoft. All rights reserved. -using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.Threading; +using System.Threading.Tasks; using Microsoft.Shared.DiagnosticIds; namespace Microsoft.Agents.AI; @@ -34,29 +35,44 @@ public abstract class AgentSkill /// /// Gets the full skill content. /// - /// + /// Cancellation token. + /// /// For file-based skills this is the raw SKILL.md file content, optionally /// augmented with a synthesized scripts block when scripts are present. /// For code-defined skills this is a synthesized XML document /// containing name, description, and body (instructions, resources, scripts). - /// - public abstract string Content { get; } + /// + public abstract ValueTask GetContentAsync(CancellationToken cancellationToken = default); /// - /// Gets the resources associated with this skill, or if none. + /// Gets a resource owned by this skill by name. /// + /// The resource name (e.g. an identifier or a relative path referenced inside the skill content). + /// Cancellation token. + /// + /// The , or when no resource with the given name exists. + /// /// - /// The default implementation returns . - /// Override this property in derived classes to provide skill-specific resources. + /// The default implementation returns . Override in derived classes that + /// expose resources. /// - public virtual IReadOnlyList? Resources => null; + public virtual ValueTask GetResourceAsync( + string name, + CancellationToken cancellationToken = default) => default; /// - /// Gets the scripts associated with this skill, or if none. + /// Gets a script owned by this skill by name. /// + /// The script name. + /// Cancellation token. + /// + /// The , or when no script with the given name exists. + /// /// - /// The default implementation returns . - /// Override this property in derived classes to provide skill-specific scripts. + /// The default implementation returns . Override in derived classes that + /// expose scripts. /// - public virtual IReadOnlyList? Scripts => null; + public virtual ValueTask GetScriptAsync( + string name, + CancellationToken cancellationToken = default) => default; } diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProvider.cs b/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProvider.cs index af1225c9df..4d9ae154f0 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProvider.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProvider.cs @@ -186,13 +186,10 @@ 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 }); - return new AIContext { - Instructions = this.BuildSkillsInstructions(skills, includeScriptInstructions: hasScripts, hasResources), - Tools = this.BuildTools(skills, hasScripts, hasResources), + Instructions = this.BuildSkillsInstructions(skills), + Tools = this.BuildTools(skills), }; } @@ -219,29 +216,20 @@ private async Task GetOrCreateContextAsync(InvokingContext context, C } } - private IList BuildTools(IList skills, bool hasScripts, bool hasResources) + private IList BuildTools(IList skills) { IList tools = [ AIFunctionFactory.Create( - (string skillName) => this.LoadSkill(skills, skillName), + (string skillName, CancellationToken cancellationToken) => this.LoadSkillAsync(skills, skillName, cancellationToken), name: "load_skill", description: "Loads the full content of a specific skill"), - ]; - - if (hasResources) - { - tools.Add(AIFunctionFactory.Create( + AIFunctionFactory.Create( (string skillName, string resourceName, IServiceProvider? serviceProvider, CancellationToken cancellationToken = default) => this.ReadSkillResourceAsync(skills, skillName, resourceName, serviceProvider, cancellationToken), name: "read_skill_resource", - description: "Reads a resource associated with a skill, such as references, assets, or dynamic data.")); - } - - if (!hasScripts) - { - return tools; - } + description: "Reads a resource associated with a skill, such as references, assets, or dynamic data."), + ]; AIFunction scriptFunction = AIFunctionFactory.Create( (string skillName, string scriptName, JsonElement? arguments = null, IServiceProvider? serviceProvider = null, CancellationToken cancellationToken = default) => @@ -257,7 +245,7 @@ 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,32 +258,29 @@ 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(); } - private string LoadSkill(IList skills, string skillName) + private async Task LoadSkillAsync(IList skills, string skillName, CancellationToken cancellationToken) { if (string.IsNullOrWhiteSpace(skillName)) { return "Error: Skill name cannot be empty."; } - var skill = skills?.FirstOrDefault(skill => skill.Frontmatter.Name == skillName); + var skill = skills.FirstOrDefault(skill => skill.Frontmatter.Name == skillName); if (skill == null) { return $"Error: Skill '{skillName}' not found."; @@ -303,7 +288,7 @@ 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 +303,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,20 +338,20 @@ 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); - if (script is null) - { - return $"Error: Script '{scriptName}' not found in skill '{skillName}'."; - } - try { + var script = await skill.GetScriptAsync(scriptName, cancellationToken).ConfigureAwait(false); + if (script is null) + { + return $"Error: Script '{scriptName}' not found in skill '{skillName}'."; + } + return await script.RunAsync(skill, arguments, serviceProvider, cancellationToken).ConfigureAwait(false); } catch (Exception ex) diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkill.cs b/dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkill.cs index 3e10557968..d1c792de21 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkill.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkill.cs @@ -2,6 +2,9 @@ using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; using Microsoft.Shared.DiagnosticIds; using Microsoft.Shared.Diagnostics; @@ -50,11 +53,12 @@ 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,16 @@ public override string Content public string Path { get; } /// - public override IReadOnlyList Resources => this._resources; + public override ValueTask GetResourceAsync(string name, CancellationToken cancellationToken = default) + { + var resource = this._resources.FirstOrDefault(r => r.Name == name); + return new(resource); + } /// - public override IReadOnlyList Scripts => this._scripts; + public override ValueTask GetScriptAsync(string name, CancellationToken cancellationToken = default) + { + var script = this._scripts.FirstOrDefault(s => s.Name == name); + return new(script); + } } diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentClassSkill.cs b/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentClassSkill.cs index b44f423bc2..32f461e32a 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentClassSkill.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentClassSkill.cs @@ -4,9 +4,11 @@ using System.Collections.Generic; using System.ComponentModel; using System.Diagnostics.CodeAnalysis; +using System.Linq; using System.Reflection; using System.Text.Json; using System.Threading; +using System.Threading.Tasks; using Microsoft.Extensions.AI; using Microsoft.Shared.DiagnosticIds; @@ -34,9 +36,9 @@ namespace Microsoft.Agents.AI; /// discovered via reflection on . This approach is compatible with Native AOT. /// /// -/// Explicit override: Override and , using -/// , , -/// and to define inline resources and scripts. This approach is also compatible with Native AOT. +/// Explicit override: Override and , using , +/// , and to define +/// inline resources and scripts. This approach is also compatible with Native AOT. /// /// /// @@ -97,11 +99,24 @@ public abstract class AgentClassSkill< { private const BindingFlags DiscoveryBindingFlags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static; - private string? _content; - private bool _resourcesDiscovered; - private bool _scriptsDiscovered; - private IReadOnlyList? _reflectedResources; - private IReadOnlyList? _reflectedScripts; + private readonly Lazy?> _resources; + private readonly Lazy?> _scripts; + private readonly Lazy _content; + + /// + /// Initializes a new instance of the class. + /// + protected AgentClassSkill() + { + this._resources = new Lazy?>(this.DiscoverResources); + this._scripts = new Lazy?>(this.DiscoverScripts); + this._content = new Lazy(() => AgentInlineSkillContentBuilder.Build( + this.Frontmatter.Name, + this.Frontmatter.Description, + this.Instructions, + this.Resources, + this.Scripts)); + } /// /// Gets the raw instructions text for this skill. @@ -126,53 +141,44 @@ public abstract class AgentClassSkill< /// Returns a synthesized XML document containing name, description, instructions, resources, and scripts. /// The result is cached after the first access. Override to provide custom content. /// - public override string Content => this._content ??= AgentInlineSkillContentBuilder.Build( - this.Frontmatter.Name, - this.Frontmatter.Description, - this.Instructions, - this.Resources, - this.Scripts); + public override ValueTask GetContentAsync(CancellationToken cancellationToken = default) => new(this._content.Value); - /// + /// + /// Gets the resources associated with this skill, or if none. + /// /// - /// 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 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..4fb2b045cb 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentInlineSkill.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentInlineSkill.cs @@ -3,7 +3,10 @@ using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.Linq; using System.Text.Json; +using System.Threading; +using System.Threading.Tasks; using Microsoft.Extensions.AI; using Microsoft.Shared.DiagnosticIds; using Microsoft.Shared.Diagnostics; @@ -16,9 +19,9 @@ namespace Microsoft.Agents.AI; /// /// All calls to , /// , and -/// must be made before the skill's is first accessed. +/// must be made before the skill's is first called. /// Calls made after that point will not be reflected in the generated -/// . In typical usage, this means configuring all +/// content. In typical usage, this means configuring all /// resources and scripts before registering the skill with an /// or . /// @@ -90,13 +93,24 @@ public 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 IReadOnlyList? Resources => this._resources; + public override ValueTask GetResourceAsync(string name, CancellationToken cancellationToken = default) + { + var resource = this._resources?.FirstOrDefault(r => r.Name == name); + return new(resource); + } /// - public override IReadOnlyList? Scripts => this._scripts; + public override ValueTask GetScriptAsync(string name, CancellationToken cancellationToken = default) + { + var script = this._scripts?.FirstOrDefault(s => s.Name == name); + return new(script); + } /// /// Registers a static resource with this skill. diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentSkillResourceAttribute.cs b/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentSkillResourceAttribute.cs index a642d6c281..7094ab57ce 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentSkillResourceAttribute.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentSkillResourceAttribute.cs @@ -27,7 +27,7 @@ namespace Microsoft.Agents.AI; /// /// /// This attribute is compatible with Native AOT when used with . -/// Alternatively, override the property and use +/// Alternatively, override and use /// instead. /// /// diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentSkillScriptAttribute.cs b/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentSkillScriptAttribute.cs index 30f65cf383..afb76c108d 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentSkillScriptAttribute.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentSkillScriptAttribute.cs @@ -26,7 +26,7 @@ namespace Microsoft.Agents.AI; /// /// /// This attribute is compatible with Native AOT when used with . -/// Alternatively, override the property and use +/// Alternatively, override and use /// instead. /// /// diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentClassSkillTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentClassSkillTests.cs index dc83fad119..1248866a52 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentClassSkillTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentClassSkillTests.cs @@ -18,7 +18,7 @@ namespace Microsoft.Agents.AI.UnitTests.AgentSkills; public sealed class AgentClassSkillTests { [Fact] - public void MinimalClassSkill_HasNullOverrides_AndSynthesizesContent() + public async Task MinimalClassSkill_HasNullOverrides_AndSynthesizesContentAsync() { // Arrange var skill = new MinimalClassSkill(); @@ -26,18 +26,17 @@ public void MinimalClassSkill_HasNullOverrides_AndSynthesizesContent() // Act & Assert — null overrides Assert.Equal("minimal", skill.Frontmatter.Name); Assert.Null(skill.Resources); - Assert.Null(skill.Scripts); // Act & Assert — synthesized XML content - Assert.Contains("minimal", skill.Content); - Assert.Contains("A minimal skill.", skill.Content); - Assert.Contains("", skill.Content); - Assert.Contains("Minimal skill body.", skill.Content); - Assert.Contains("", skill.Content); + Assert.Contains("minimal", await skill.GetContentAsync()); + Assert.Contains("A minimal skill.", await skill.GetContentAsync()); + Assert.Contains("", await skill.GetContentAsync()); + Assert.Contains("Minimal skill body.", await skill.GetContentAsync()); + Assert.Contains("", await skill.GetContentAsync()); } [Fact] - public void FullClassSkill_ReturnsOverriddenLists_AndCachesContent() + public async Task FullClassSkill_ReturnsOverriddenLists_AndCachesContentAsync() { // Arrange var skill = new FullClassSkill(); @@ -50,11 +49,11 @@ public 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] @@ -117,6 +116,116 @@ public void PartialOverrides_OneCollectionNull_OtherHasValues() Assert.Single(scriptOnly.Scripts!); } + [Fact] + public async Task GetResourceAsync_ExistingName_ReturnsResourceAsync() + { + // Arrange + var skill = new FullClassSkill(); + + // Act + var resource = await skill.GetResourceAsync("test-resource"); + + // Assert + Assert.NotNull(resource); + Assert.Equal("test-resource", resource!.Name); + } + + [Fact] + public async Task GetResourceAsync_NonExistingName_ReturnsNullAsync() + { + // Arrange + var skill = new FullClassSkill(); + + // Act + var resource = await skill.GetResourceAsync("missing"); + + // Assert + Assert.Null(resource); + } + + [Fact] + public async Task GetResourceAsync_NoResources_ReturnsNullAsync() + { + // Arrange + var skill = new MinimalClassSkill(); + + // Act + var resource = await skill.GetResourceAsync("anything"); + + // Assert + Assert.Null(resource); + } + + [Fact] + public async Task GetScriptAsync_ExistingName_ReturnsScriptAsync() + { + // Arrange + var skill = new FullClassSkill(); + + // Act + var script = await skill.GetScriptAsync("TestScript"); + + // Assert + Assert.NotNull(script); + Assert.Equal("TestScript", script!.Name); + } + + [Fact] + public async Task GetScriptAsync_NonExistingName_ReturnsNullAsync() + { + // Arrange + var skill = new FullClassSkill(); + + // Act + var script = await skill.GetScriptAsync("missing"); + + // Assert + Assert.Null(script); + } + + [Fact] + public async Task GetScriptAsync_NoScripts_ReturnsNullAsync() + { + // Arrange + var skill = new MinimalClassSkill(); + + // Act + var script = await skill.GetScriptAsync("anything"); + + // Assert + Assert.Null(script); + } + + [Fact] + public async Task ConcurrentAccess_ToReflectedResourcesScriptsAndContent_InvokesDiscoveryOnceAsync() + { + // Regression test for thread-safety of Lazy initialization in AgentClassSkill. + // AttributedFullSkill uses attribute-based discovery (no override), so it exercises + // the base class's Lazy fields rather than a subclass's own caching. + var skill = new AttributedFullSkill(); + + const int Concurrency = 32; + var resourcesResults = new IReadOnlyList?[Concurrency]; + var scriptsResults = new IReadOnlyList?[Concurrency]; + var contentResults = new string[Concurrency]; + + // Act — invoke all three accessors concurrently from many threads. + await Task.WhenAll(Enumerable.Range(0, Concurrency).Select(i => Task.Run(async () => + { + resourcesResults[i] = skill.Resources; + scriptsResults[i] = skill.Scripts; + contentResults[i] = await skill.GetContentAsync(); + }))); + + // Assert — every thread observed the same cached instances (no torn state). + for (int i = 1; i < Concurrency; i++) + { + Assert.Same(resourcesResults[0], resourcesResults[i]); + Assert.Same(scriptsResults[0], scriptsResults[i]); + Assert.Same(contentResults[0], contentResults[i]); + } + } + [Fact] public async Task CreateScriptAndResource_WithSerializerOptions_HandleCustomTypesAsync() { @@ -151,17 +260,14 @@ public void Scripts_DiscoveredViaAttribute_WithCorrectNamesAndDescriptions() // 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 +378,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,33 +405,33 @@ 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] @@ -382,7 +488,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 +504,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..9a27528051 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillScriptTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillScriptTests.cs @@ -105,7 +105,7 @@ public void ParametersSchema_ReturnsExpectedArraySchema() } [Fact] - public void Content_WithScripts_AppendsPerScriptEntries() + public async Task Content_WithScripts_AppendsPerScriptEntriesAsync() { // Arrange static Task RunnerAsync(AgentFileSkill s, AgentFileSkillScript sc, JsonElement? a, IServiceProvider? sp, CancellationToken ct) => Task.FromResult(null); @@ -118,7 +118,7 @@ public 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_ContainsDefaultParametersSchemaAsync() { // 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..32e018ae90 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillsSourceScriptTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillsSourceScriptTests.cs @@ -2,7 +2,6 @@ using System; using System.IO; -using System.Linq; using System.Text.Json; using System.Threading; using System.Threading.Tasks; @@ -46,9 +45,9 @@ public 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); + var script = await skill.GetScriptAsync("scripts/convert.py"); + Assert.NotNull(script); + Assert.Equal("scripts/convert.py", script!.Name); } [Fact] @@ -69,14 +68,13 @@ 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 + foreach (var name in (string[])["scripts/run.cs", "scripts/run.csx", "scripts/run.js", "scripts/run.ps1", "scripts/run.py", "scripts/run.sh"]) + { + var script = await skills[0].GetScriptAsync(name); + Assert.NotNull(script); + Assert.Equal(name, script!.Name); + } } [Fact] @@ -94,7 +92,7 @@ public async Task GetSkillsAsync_NonScriptExtensionsAreNotDiscoveredAsync() // Assert Assert.Single(skills); - Assert.Empty(skills[0].Scripts!); + Assert.Null(await skills[0].GetScriptAsync("scripts/data.txt")); } [Fact] @@ -109,8 +107,7 @@ public async Task GetSkillsAsync_NoScriptFiles_ReturnsEmptyScriptsAsync() // Assert Assert.Single(skills); - Assert.NotNull(skills[0].Scripts); - Assert.Empty(skills[0].Scripts!); + Assert.Null(await skills[0].GetScriptAsync("any-script")); } [Fact] @@ -128,7 +125,7 @@ public 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.Null(await skills[0].GetScriptAsync("convert.py")); } [Fact] @@ -150,7 +147,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 +172,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 +192,9 @@ 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); + var rbScript = await skills[0].GetScriptAsync("scripts/run.rb"); + Assert.NotNull(rbScript); + Assert.Equal("scripts/run.rb", rbScript!.Name); } [Fact] @@ -217,7 +215,7 @@ public 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 +238,9 @@ 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); + var nestedScript = await skills[0].GetScriptAsync("f1/f2/f3/run.py"); + Assert.NotNull(nestedScript); + Assert.Equal("f1/f2/f3/run.py", nestedScript!.Name); } [Theory] @@ -267,11 +266,12 @@ public 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); foreach (string directory in directories) { string expectedName = $"{directory.Substring(2)}/run.py"; - Assert.Contains(skills[0].Scripts!, s => s.Name == expectedName); + var script = await skills[0].GetScriptAsync(expectedName); + Assert.NotNull(script); + Assert.Equal(expectedName, script!.Name); } } diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentInlineSkillTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentInlineSkillTests.cs index 817223bac7..c6ae2212bf 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentInlineSkillTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentInlineSkillTests.cs @@ -105,13 +105,13 @@ public 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,17 +280,103 @@ 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] - public void Scripts_WhenNoneAdded_ReturnsNull() + public async Task Scripts_WhenNoneAdded_ReturnsNullAsync() { // Arrange var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); // Act & Assert - Assert.Null(skill.Scripts); + Assert.Null(await skill.GetScriptAsync("nonexistent")); + } + + [Fact] + public async Task GetResourceAsync_ExistingName_ReturnsResourceAsync() + { + // Arrange + var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); + skill.AddResource("r1", "v1"); + skill.AddResource("r2", "v2"); + + // Act + var resource = await skill.GetResourceAsync("r2"); + + // Assert + Assert.NotNull(resource); + Assert.Equal("r2", resource!.Name); + } + + [Fact] + public async Task GetResourceAsync_NonExistingName_ReturnsNullAsync() + { + // Arrange + var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); + skill.AddResource("r1", "v1"); + + // Act + var resource = await skill.GetResourceAsync("missing"); + + // Assert + Assert.Null(resource); + } + + [Fact] + public async Task GetResourceAsync_NoResourcesAdded_ReturnsNullAsync() + { + // Arrange + var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); + + // Act + var resource = await skill.GetResourceAsync("missing"); + + // Assert + Assert.Null(resource); + } + + [Fact] + public async Task GetScriptAsync_ExistingName_ReturnsScriptAsync() + { + // Arrange + var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); + skill.AddScript("s1", () => "first"); + skill.AddScript("s2", () => "second"); + + // Act + var script = await skill.GetScriptAsync("s2"); + + // Assert + Assert.NotNull(script); + Assert.Equal("s2", script!.Name); + } + + [Fact] + public async Task GetScriptAsync_NonExistingName_ReturnsNullAsync() + { + // Arrange + var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); + skill.AddScript("s1", () => "ok"); + + // Act + var script = await skill.GetScriptAsync("missing"); + + // Assert + Assert.Null(script); + } + + [Fact] + public async Task GetScriptAsync_NoScriptsAdded_ReturnsNullAsync() + { + // Arrange + var skill = new AgentInlineSkill("my-skill", "A valid skill.", "Instructions."); + + // Act + var script = await skill.GetScriptAsync("missing"); + + // Assert + Assert.Null(script); } [Fact] @@ -333,13 +419,13 @@ public 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 +433,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 +492,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 +500,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 +523,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 +547,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 +563,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 +580,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..5a67bcf62c --- /dev/null +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillTestExtensions.cs @@ -0,0 +1,43 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.Collections.Generic; +using System.Linq; +using System.Reflection; + +namespace Microsoft.Agents.AI.UnitTests.AgentSkills; + +/// +/// Test-only helpers that peek at the underlying resource list of a skill via reflection. +/// +/// +/// The public API exposes resources only through +/// . +/// These helpers exist purely to allow unit tests for and +/// to inspect the concrete enumerated list a skill carries. +/// +internal static class AgentSkillTestExtensions +{ + public static IReadOnlyList? GetTestResources(this AgentSkill skill) + { + // AgentFileSkill / AgentInlineSkill: private "_resources" field. + for (var type = skill.GetType(); type is not null; type = type.BaseType) + { + var field = type.GetField("_resources", BindingFlags.NonPublic | BindingFlags.Instance); + if (field is not null) + { + return UnwrapList(field.GetValue(skill)); + } + } + + return null; + } + + private static IReadOnlyList? UnwrapList(object? value) => + value switch + { + null => null, + IReadOnlyList list => list, + IEnumerable seq => seq.ToList(), + _ => null, + }; +} diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillsProviderTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillsProviderTests.cs index 123be17387..25481ea501 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillsProviderTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillsProviderTests.cs @@ -68,11 +68,12 @@ public 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] @@ -416,7 +419,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 +448,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 +1274,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..cd362ce64e 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs @@ -281,9 +281,9 @@ public 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.Null(await skill.GetScriptAsync("any-script")); } [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,9 @@ 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); + var script = await skills[0].GetScriptAsync("scripts/run.py"); + Assert.NotNull(script); + Assert.Equal("scripts/run.py", script!.Name); } [Theory] @@ -1093,8 +1094,8 @@ public 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 +1118,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 +1189,9 @@ 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); + var script = await skill.GetScriptAsync("run.py"); + Assert.NotNull(script); + Assert.Equal("run.py", script!.Name); } #if NET @@ -1229,8 +1231,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/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] diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/TestSkillTypes.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/TestSkillTypes.cs index a2fb4155e4..7110fb438b 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/TestSkillTypes.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/TestSkillTypes.cs @@ -31,13 +31,7 @@ public TestAgentSkill(string name, string description, string content) public override AgentSkillFrontmatter Frontmatter => this._frontmatter; /// - public override string Content => this._content; - - /// - public override IReadOnlyList? Resources => null; - - /// - public override IReadOnlyList? Scripts => null; + public override ValueTask GetContentAsync(CancellationToken cancellationToken = default) => new(this._content); } ///