.NET/Python: Refine skill frontmatter parsing - #8430
Evan Mattson (moonbox3) merged 6 commits into
Conversation
Validate recognized top-level field casing and uniqueness while preserving existing scalar parsing and nested metadata behavior. Add file and MCP archive regression coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Warn on duplicate metadata entries without rejecting skills, preserve existing key comparison rules, and expand scalar and archive regression coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep bare empty optional fields null while retaining duplicate and casing validation in Python and .NET. Add file and MCP archive regression coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟢 Approval recommended
The implementations match the described behavior and include comprehensive cross-language coverage.
Pull request overview
Refines Python and .NET SKILL.md parsing while preserving implementation-specific metadata behavior.
Changes:
- Rejects duplicate or incorrectly cased recognized root fields.
- Retains first duplicate metadata values and logs warnings.
- Adds extensive file and MCP archive coverage.
File summaries
| File | Description |
|---|---|
python/packages/core/agent_framework/_skills.py |
Refines Python frontmatter parsing. |
python/packages/core/tests/core/test_skills.py |
Covers Python file-based parsing. |
python/packages/core/tests/core/test_mcp_skills.py |
Covers Python archive parsing. |
dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkillsSource.cs |
Refines .NET frontmatter parsing. |
dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillFrontmatter.cs |
Documents metadata behavior. |
dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs |
Covers .NET file-based parsing. |
dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceArchiveTests.cs |
Covers .NET archive parsing. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 0
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (3 commit(s)): e2371ea6e928, 85b7fc33f69e, 40788a7e0882
Model: gpt-5.6-sol-fast
Overview
The PR consistently applies canonical root-field checks and first-value metadata handling to local-file and MCP archive loading, with broad parity and scalar-regression coverage in both implementations. Validation-before-assignment and shared parser call paths are strong guardrails. One residual gap remains: valid YAML quoted keys bypass the new recognized-field casing and duplicate checks in both languages.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
1 verified finding remained after source verification (1 medium) across 1 file. Details are attached to the affected lines below.
Affected areas: python/packages/core/agent_framework/_skills.py
Motivation & Context
Define explicit handling of root properties and nested metadata items in
SKILL.mdfrontmatter, while preserving each implementation's existing metadata lookup conventions.Description & Review Guide
What are the major changes?
metadataproperty. Unknown properties remain ignored. Empty declarations still undergo these checks, but bare optional scalar properties remain unset (None/null).What is the impact of these changes? These rules apply to local-file and MCP archive loading. Public APIs, metadata dictionary types, and existing supported scalar parsing behavior remain unchanged.
Related Issue
N/A.
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.