.NET: [Breaking] Restructure agent skills to use multi-source architecture#4871
.NET: [Breaking] Restructure agent skills to use multi-source architecture#4871SergeyMenshykh wants to merge 8 commits intomicrosoft:mainfrom
Conversation
f628bb8 to
0e78d14
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors the .NET Agent Skills system to a multi-source architecture (per ADR-0021), introducing composable skill sources (file, in-memory, aggregate) plus decorators (filtering, caching, deduplication), and updates samples/tests accordingly.
Changes:
- Introduces new skills abstractions:
AgentSkillsSource,AgentSkill*models, andAgentSkillsProvider+ fluentAgentSkillsProviderBuilder. - Adds file-based discovery via
AgentFileSkillsSource(resources + scripts) and decorator sources for filtering/dedup/caching. - Updates unit tests and the AgentSkills sample to the new APIs (including a subprocess script executor + unit-converter skill).
Reviewed changes
Copilot reviewed 49 out of 49 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/TestSkillTypes.cs | Adds in-memory test implementations for AgentSkill and AgentSkillsSource. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FilteringAgentSkillsSourceTests.cs | Adds coverage for filtering decorator behavior. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillsProviderTests.cs | Removes tests for deprecated FileAgentSkillsProvider. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs | Migrates loader tests to AgentFileSkillsSource (discovery/parsing/resources). |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/DeduplicatingAgentSkillsSourceTests.cs | Adds coverage for name-based deduplication decorator. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/CachingAgentSkillsSourceTests.cs | Adds coverage for caching decorator, including concurrency expectations. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AggregateAgentSkillsSourceTests.cs | Adds coverage for aggregating multiple sources. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillsProviderTests.cs | Adds end-to-end tests for AgentSkillsProvider + builder + scripts/tools. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillsProviderBuilderTests.cs | Adds tests validating builder composition and options behavior. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillFrontmatterValidatorTests.cs | Adds tests for AgentSkillFrontmatter validation rules. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillsSourceScriptTests.cs | Adds tests for file script discovery/execution plumbing. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillScriptTests.cs | Adds tests for AgentFileSkillScript execution validation/delegation. |
| dotnet/src/Microsoft.Agents.AI/Skills/SkillFrontmatter.cs | Removes old frontmatter model. |
| dotnet/src/Microsoft.Agents.AI/Skills/FileAgentSkillsProviderOptions.cs | Removes options for deprecated provider. |
| dotnet/src/Microsoft.Agents.AI/Skills/FileAgentSkillsProvider.cs | Removes old file-only provider implementation. |
| dotnet/src/Microsoft.Agents.AI/Skills/FileAgentSkill.cs | Removes old file skill model. |
| dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkillsSourceOptions.cs | Adds options container for file source discovery customization. |
| dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkillsSource.cs | Implements file-based skill discovery/parsing + resource/script enumeration. |
| dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkillScriptExecutor.cs | Introduces delegate contract for executing file-based scripts. |
| dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkillScript.cs | Implements file-based script wrapper delegating execution to an executor. |
| dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkillResource.cs | Implements file-backed resource reader. |
| dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkill.cs | Implements file-backed AgentSkill type. |
| dotnet/src/Microsoft.Agents.AI/Skills/Decorators/FilteringAgentSkillsSource.cs | Adds predicate-based filtering decorator. |
| dotnet/src/Microsoft.Agents.AI/Skills/Decorators/DelegatingAgentSkillsSource.cs | Adds base decorator class for skill sources. |
| dotnet/src/Microsoft.Agents.AI/Skills/Decorators/DeduplicatingAgentSkillsSource.cs | Adds first-wins, case-insensitive dedup decorator. |
| dotnet/src/Microsoft.Agents.AI/Skills/Decorators/CachingAgentSkillsSource.cs | Adds caching decorator for skill lists. |
| dotnet/src/Microsoft.Agents.AI/Skills/AggregateAgentSkillsSource.cs | Adds source that concatenates multiple sources in registration order. |
| dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsSource.cs | Adds base abstraction for skill discovery sources. |
| dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProviderOptions.cs | Adds provider configuration (prompt template + script approval). |
| dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProviderBuilder.cs | Adds fluent builder for composing sources + decorators + provider options. |
| dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProvider.cs | Adds new provider that exposes tools/prompts for skills/resources/scripts. |
| dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillScript.cs | Adds base abstraction for skill scripts. |
| dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillResource.cs | Adds base abstraction for skill resources. |
| dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillFrontmatter.cs | Adds validated frontmatter model (name/description + optional fields). |
| dotnet/src/Microsoft.Agents.AI/Skills/AgentSkill.cs | Adds base abstraction for skills. |
| dotnet/samples/02-agents/AgentSkills/SubprocessScriptExecutor.cs | Adds sample subprocess-based executor for file scripts. |
| dotnet/samples/02-agents/AgentSkills/README.md | Updates sample index to new file-based skills sample name. |
| dotnet/samples/02-agents/AgentSkills/Agent_Step01_FileBasedSkills/skills/unit-converter/scripts/convert.py | Adds a sample Python conversion script. |
| dotnet/samples/02-agents/AgentSkills/Agent_Step01_FileBasedSkills/skills/unit-converter/references/conversion-table.md | Adds sample reference data for unit conversion. |
| dotnet/samples/02-agents/AgentSkills/Agent_Step01_FileBasedSkills/skills/unit-converter/SKILL.md | Adds sample file-based skill definition. |
| dotnet/samples/02-agents/AgentSkills/Agent_Step01_FileBasedSkills/README.md | Adds documentation for running the new file-based skills sample. |
| dotnet/samples/02-agents/AgentSkills/Agent_Step01_FileBasedSkills/Program.cs | Adds new sample program wired to AgentSkillsProviderBuilder. |
| dotnet/samples/02-agents/AgentSkills/Agent_Step01_FileBasedSkills/Agent_Step01_FileBasedSkills.csproj | Links in SubprocessScriptExecutor and references agent framework projects. |
| dotnet/samples/02-agents/AgentSkills/Agent_Step01_BasicSkills/skills/expense-report/references/POLICY_FAQ.md | Removes legacy expense-report sample content. |
| dotnet/samples/02-agents/AgentSkills/Agent_Step01_BasicSkills/skills/expense-report/assets/expense-report-template.md | Removes legacy expense-report sample asset. |
| dotnet/samples/02-agents/AgentSkills/Agent_Step01_BasicSkills/skills/expense-report/SKILL.md | Removes legacy expense-report skill definition. |
| dotnet/samples/02-agents/AgentSkills/Agent_Step01_BasicSkills/README.md | Removes legacy sample README. |
| dotnet/samples/02-agents/AgentSkills/Agent_Step01_BasicSkills/Program.cs | Removes legacy sample program using old provider. |
| dotnet/agent-framework-dotnet.slnx | Updates solution sample project path to new sample. |
| dotnet/0021-agent-skills-design.md | Adds ADR/design doc describing the multi-source skills architecture. |
Comments suppressed due to low confidence (1)
dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkillsSource.cs:507
ValidateExtensionsthrows anArgumentExceptionwith paramName "allowedResourceExtensions" regardless of whether resource or script extensions are being validated. This is misleading for callers and makes debugging harder. Pass a correct parameter name (e.g., the property name being validated) intoValidateExtensions, or usenameof(AgentFileSkillsSourceOptions.AllowedResourceExtensions)/AllowedScriptExtensionsas appropriate.
dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs
Outdated
Show resolved
Hide resolved
dotnet/samples/02-agents/AgentSkills/SubprocessScriptExecutor.cs
Outdated
Show resolved
Hide resolved
dotnet/src/Microsoft.Agents.AI/Skills/Decorators/CachingAgentSkillsSource.cs
Outdated
Show resolved
Hide resolved
dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/DeduplicatingAgentSkillsSourceTests.cs
Outdated
Show resolved
Hide resolved
dotnet/src/Microsoft.Agents.AI/Skills/AggregateAgentSkillsSource.cs
Outdated
Show resolved
Hide resolved
| /// to CLI flags, and returns captured output. It is intended for | ||
| /// demonstration purposes only. | ||
| /// </remarks> | ||
| internal static class SubprocessScriptRunner |
There was a problem hiding this comment.
I'm not fully found of "hiding" this complexity from the samples is there any way of having a simpler/slim version of the runner for the individual samples?
There was a problem hiding this comment.
We can't compress it into a few lines, unfortunately, and will have to include it in each sample. There will be at least one introduced in a follow-up PR.
| /// <param name="loggerFactory">Optional logger factory.</param> | ||
| public AgentSkillsProvider( | ||
| string skillPath, | ||
| AgentFileSkillScriptRunner scriptRunner, |
There was a problem hiding this comment.
Does this need to be required, if the skill has no scripts?
There was a problem hiding this comment.
It was optional, but during one of the reviews, it was suggested to make it required. I can make it optional to support cases where skills have no scripts, and throw an exception if skills have scripts but no runner is provided.
| IReadOnlyList<AgentSkillScript>? scripts = null) | ||
| { | ||
| this.Frontmatter = Throw.IfNull(frontmatter); | ||
| this.Content = Throw.IfNull(content); |
There was a problem hiding this comment.
Should we allow empty / white spaced skills?
| this.Content = Throw.IfNull(content); | |
| this.Content = Throw.IfNullOrWhitespace(content); |
| public override AgentSkillFrontmatter Frontmatter { get; } | ||
|
|
||
| /// <inheritdoc/> | ||
| public override string Content { get; } |
There was a problem hiding this comment.
Do we have a strong case to make those abstract instead of virtual in base.
If not having the flexibility by default being plain getter might be better thoughts?
| public override string Content { get; } |
| /// <inheritdoc/> | ||
| #pragma warning disable CA1725 // Parameter names should match base declaration | ||
| #pragma warning disable RCS1168 // Parameter name differs from base name | ||
| public override async Task<object?> ReadAsync(IServiceProvider? _, CancellationToken cancellationToken = default) | ||
| #pragma warning restore RCS1168 // Parameter name differs from base name | ||
| #pragma warning restore CA1725 // Parameter names should match base declaration |
There was a problem hiding this comment.
Might just worth using the base param name than adding 4 pragma lines around it, do we have any real IL gains with this change?
| /// <inheritdoc/> | |
| #pragma warning disable CA1725 // Parameter names should match base declaration | |
| #pragma warning disable RCS1168 // Parameter name differs from base name | |
| public override async Task<object?> ReadAsync(IServiceProvider? _, CancellationToken cancellationToken = default) | |
| #pragma warning restore RCS1168 // Parameter name differs from base name | |
| #pragma warning restore CA1725 // Parameter names should match base declaration | |
| /// <inheritdoc/> | |
| public override async Task<object?> ReadAsync(IServiceProvider? baseName, CancellationToken cancellationToken = default) |
Summary
Restructures the .NET agent skills system to use the new multi-source architecture as proposed in ADR-0021. This introduces the sources building blocks — a set of composable abstractions for discovering, filtering, caching, and deduplicating skills from multiple origins.
Key Changes
New abstractions
AgentSkillsSource— abstract base for skill discovery from any origin (filesystem, in-memory, custom)AgentSkillResource/AgentSkillScript— abstract base types for resources and scriptsAgentSkillFrontmatter— dedicated model for skill metadata (name, description, license, compatibility, etc.)Source implementations
AgentFileSkillsSource— discovers file-based skills fromSKILL.mdfiles on disk (replacesFileAgentSkillsProviderdiscovery logic)AgentInMemorySkillsSource— holds anyAgentSkillinstances in memory (for inline/class-based skills)AggregateAgentSkillsSource— composes multiple sources into oneSource decorators
DelegatingAgentSkillsSource— abstract base for interceptingGetSkillsAsync()FilteringAgentSkillsSource— applies predicate-based skill filteringCachingAgentSkillsSource— caches skills after first loadDeduplicatingAgentSkillsSource— deduplicates by name (case-insensitive, first-wins)File-based skill types
AgentFileSkill— replacesFileAgentSkillwith clearer naming and source-oriented designAgentFileSkillResource— reads content from files on diskAgentFileSkillScript— delegates execution to a configurableAgentFileSkillScriptExecutorProvider and builder
AgentSkillsProvider— updated to accept a singleAgentSkillsSource(composed externally)AgentSkillsProviderBuilder— fluent API for composing skills from multiple source typesSample updates
Agent_Step01_BasicSkills→Agent_Step01_FileBasedSkillswith updated skill contentSubprocessScriptExecutorhelper for running external scriptsRelated