.NET: Explicitly emit available_resources and available_scripts in skill content - #6672
Conversation
…ill content AgentInlineSkillContentBuilder now always emits <available_resources> and <available_scripts> elements, using self-closing tags when a skill has no resources or scripts. This signals to the model exactly what is callable so it does not hallucinate non-existent resource or script names. Script parameter schemas are wrapped in a nested <parameters_schema> element. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Automated Code Review
Reviewers: 5 | Confidence: 90%
✓ Correctness
The PR is well-implemented with no correctness issues found. The builder properly emits self-closing XML tags for empty resources/scripts and populated blocks otherwise. XML escaping is consistently applied. The AgentFileSkill intentionally uses different behavior (appending only the scripts block from BuildAvailableScriptsBlock without the empty-element fallbacks) which is appropriate since file-based skills have human-authored SKILL.md content. All code paths are covered by comprehensive tests.
✓ Security Reliability
The PR is clean from a security and reliability standpoint. XML special characters in resource and script names are properly escaped before interpolation into attribute values. The escaping function processes ampersands first (preventing double-encoding), and validates all required inputs. No injection risks, resource leaks, or unsafe patterns were identified.
✓ Test Coverage
The test coverage for this PR is strong. The new
AgentInlineSkillContentBuilderTestsfile provides thorough unit testing of the builder logic covering null/empty inputs, resources-only, scripts-only, ordering, XML escaping, and error cases. The existing integration tests inAgentInlineSkillTests,AgentClassSkillTests, andAgentFileSkillScriptTestsare all properly updated to verify the new element names and behavior. The one minor gap is a missing direct unit test forBuildwith both resources and scripts populated simultaneously (tested indirectly throughAgentInlineSkillTests.Content_IncludesScriptSchemasAddedBeforeFirstAccessAsync). TheAgentFileSkillintentionally doesn't emit<available_resources>since file skills have human-authored SKILL.md content, which is a reasonable design choice.
✓ Failure Modes
This PR cleanly refactors the XML output format for skill content (renaming elements, always emitting available_resources/available_scripts blocks). No silent failures, lost errors, or operational failure modes are introduced. The null/empty handling is correct—null and empty collections both emit self-closing tags. XML special characters in names are properly escaped for attribute values. The pre-existing caching patterns (Lazy in AgentClassSkill, ??= in AgentInlineSkill) are unchanged and remain thread-safe-enough for their use case (worst case: duplicate string computation, no data loss). AgentFileSkill's omission of resource/empty-script markers is a pre-existing design choice for file-backed skills and is not regressed by this diff.
✓ Design Approach
The PR fixes discoverability for programatic skills, but it leaves the same design problem in file-backed skills: discovered resources are still callable through
read_skill_resourcewithout being surfaced in the loaded skill content, so models have no authoritative list of valid resource names.
Automated review by SergeyMenshykh's agents
There was a problem hiding this comment.
Pull request overview
This PR updates the .NET skill content generation to always emit explicit <available_resources> and <available_scripts> sections (self-closing when empty), and refines the script schema shape by nesting it under <parameters_schema>.
Changes:
- Updated
AgentInlineSkillContentBuilder.Build(...)to always include<available_resources>/<available_scripts>(self-closing when none), and to emit script entries as<script name="...">with an optional nested<parameters_schema>. - Propagated the new emitted XML shape through programmatic and file-based skill content paths, and updated XML docs accordingly.
- Added/updated unit tests to validate the new output structure and escaping behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentInlineSkillContentBuilder.cs | Implements the new always-present resources/scripts blocks and the updated script schema XML shape. |
| dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentInlineSkill.cs | Passes resources into the content builder and updates public XML docs to reflect the new emitted blocks. |
| dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentClassSkill.cs | Passes resources into the content builder and updates XML docs for class-based skill resources/scripts. |
| dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkill.cs | Renames the appended scripts block to the new <available_scripts> builder method. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentInlineSkillTests.cs | Updates assertions to match the new <available_resources> / <available_scripts> content shape. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentInlineSkillContentBuilderTests.cs | Adds focused unit tests for the new builder output structure, escaping, and schema wrapping. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillScriptTests.cs | Updates file-skill content assertions to match the new available-scripts XML block. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentClassSkillTests.cs | Updates class-skill content assertions to match the new emitted resources/scripts blocks. |
There was a problem hiding this comment.
Automated Code Review
Reviewers: 5 | Confidence: 92%
✓ Correctness
The PR is correct. The content builder properly emits self-closing XML elements for inline/class skills when no resources or scripts exist, and file skills correctly delegate to the block builders which return empty string for empty lists (preserving backward-compatible behavior validated by the unchanged test). Null handling, XML escaping, and type covariance are all correct.
✓ Security Reliability
The PR is well-implemented from a security and reliability perspective. XML special characters in resource/script names are properly escaped before being placed in attribute values, preventing XML injection. Input validation via Throw.IfNull/IfNullOrWhitespace exists at all trust boundaries. The preserveQuotes path for JSON schema content is safe because it's used only in element content (not attributes), so unescaped quotes cannot break the XML structure while < and > remain escaped. No resource leaks, secrets, or unsafe deserialization patterns are introduced.
✓ Test Coverage
Test coverage for this PR is comprehensive. The new
AgentInlineSkillContentBuilderTestsclass thoroughly tests the builder's output for empty/null/populated resources and scripts, XML escaping, registration order, and argument validation. Existing test files are properly updated to reflect the new XML structure. TheFakeScriptmock is minimal and appropriate. Assertions target exact XML snippets rather than broad substring matches, reducing false-positive risk. No critical test coverage gaps found for the changed behavior.
✓ Failure Modes
No operational failure modes found. The code correctly handles null and empty cases through pattern matching and null-coalescing in constructors. The renamed method has no orphaned callers. The caching pattern is safe for concurrent access. The file-skill discoverability gap (empty resources/scripts don't emit self-closing tags) was already raised and resolved in a prior review round.
✗ Design Approach
The new XML shape is consistent for inline and class-based skills, but file-backed skills still leave one hallucination path open: when a file skill has only resources or only scripts,
GetContentAsyncomits the empty peer block entirely. That means the provider still gives the model no authoritative “none exist” signal for the missing category, even though the prompt contract says to use resource/script names exactly as listed.
Flagged Issues
-
AgentFileSkill.GetContentAsyncstill omits the empty peer block for one-sided file skills. BecauseBuildAvailableResourcesBlock/BuildAvailableScriptsBlockreturnstring.Emptyfor empty lists (AgentInlineSkillContentBuilder.cs:78-80,109-111), a file skill with only scripts or only resources produces content where one category is not listed at all.AgentSkillsProviderinstructs models to use names "exactly as listed" (Skills/AgentSkillsProvider.cs:263-267), so this path still silently withholds the authoritative "none available" signal that the PR is adding elsewhere.
Automated review by SergeyMenshykh's agents
|
Flagged issue
Source: automated DevFlow PR review |
Align AgentFileSkill with inline/class skills by surfacing discovered resources in the loaded skill content. AgentFileSkill.GetContentAsync now appends an <available_resources> block (before <available_scripts>) listing resource names so the model has an authoritative list and does not hallucinate resource names. Extracted a reusable BuildAvailableResourcesBlock helper in AgentInlineSkillContentBuilder. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
356e7b4 to
bb7fdc8
Compare
rogerbarreto
left a comment
There was a problem hiding this comment.
Would be nice having an Eval's set up for this exact scenario, so we can measure some strategies for LLM adherence
Port the .NET AgentFileSkill behavior from microsoft/agent-framework#6672 so file-backed skill content always advertises available resources and scripts, including empty self-closing blocks when none are present. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Motivation & Context
When a skill has no resources or scripts,
AgentInlineSkillContentBuilderleft those sections out of the generated content entirely. With nothing saying "there are none", models can assume resources/scripts exist and hallucinate calls to names that aren't there.Description & Review Guide
AgentInlineSkillContentBuilder.Buildnow always emits<available_resources>and<available_scripts>- listing entries by name when present, or self-closing (<available_resources />/<available_scripts />) when empty. Script schemas are wrapped in a nested<parameters_schema>element. Added builder unit tests and updated public XML docs.Related Issue
Closes: #6371
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) - a workflow keeps the label and title prefix in sync automatically.