Skip to content

[BREAKING] Python: Align file skill folder discovery with agentskills.io spec#5807

Merged
SergeyMenshykh merged 3 commits into
microsoft:mainfrom
SergeyMenshykh:semenshi/align-file-skill-discovery-with-spec
May 14, 2026
Merged

[BREAKING] Python: Align file skill folder discovery with agentskills.io spec#5807
SergeyMenshykh merged 3 commits into
microsoft:mainfrom
SergeyMenshykh:semenshi/align-file-skill-discovery-with-spec

Conversation

@SergeyMenshykh
Copy link
Copy Markdown
Member

@SergeyMenshykh SergeyMenshykh commented May 13, 2026

Motivation and Context

Port of .NET PR #5078. The agentskills.io specification defines a standard directory structure for skill files. The previous implementation used recursive rglob scanning of the entire skill directory, which does not align with the spec and could pick up unintended files.

Resolves #5711.

Description

Updates FileSkillsSource to scan spec-defined subdirectories instead of recursive rglob:

  • Resources default to references/ and assets/; scripts default to scripts/
  • Add resource_directories and script_directories parameters for customization
  • Support . root indicator for skill root files
  • Non-recursive (top-level only) scanning within each configured directory
  • Directory validation: reject .., absolute paths, empty names
  • Containment checks, symlink guards, cross-platform path handling

[BREAKING] — Constructor and discovery behavior changed. Skills with resources or scripts outside the configured directories (references/, assets/, scripts/) will no longer discover those files automatically. Use the resource_directories or script_directories parameters to include custom directories, or move files into the standard directories. Acceptable as the Skills API is @experimental.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • The PR follows the Contribution Guidelines
  • All unit tests pass, and I have added new tests where possible
  • Is this a breaking change? Yes — experimental API only. Title prefixed with [BREAKING].

Update FileSkillsSource to scan spec-defined subdirectories instead of
recursive rglob for resource and script discovery:

- Resources: scan 'references/' and 'assets/' (was: entire skill tree)
- Scripts: scan 'scripts/' (was: entire skill tree)
- Add resource_directories and script_directories parameters for
  customization, with '.' root indicator for skill root files
- Add directory validation: reject '..' traversal, absolute paths, empty
  names; normalize separators and deduplicate directories
- Non-recursive scanning within each configured directory (top-level only)
- Containment check validates files against target directory, not just
  skill root, for stronger path-traversal defense
- Case-insensitive directory deduplication via os.path.normcase()
- Cross-platform absolute path rejection in directory validation
- Sort discovery results for stable ordering
- Update SkillsProvider.from_paths() to pass new parameters through
- Update all tests for new subdirectory-scoped discovery behavior

Resolves microsoft#5711.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 13, 2026 11:15
@SergeyMenshykh SergeyMenshykh self-assigned this May 13, 2026
@SergeyMenshykh SergeyMenshykh moved this to In Review in Agent Framework May 13, 2026
@moonbox3
Copy link
Copy Markdown
Contributor

moonbox3 commented May 13, 2026

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
packages/core/agent_framework
   _skills.py8152397%507, 517, 954, 969, 971–972, 2394–2395, 2534, 2539, 2542, 2547, 2574, 2579, 2633, 2642, 2647, 2650, 2655, 2679, 2684, 2924–2925
TOTAL34015391988% 

Python Unit Test Overview

Tests Skipped Failures Errors Time
6733 30 💤 0 ❌ 0 🔥 1m 47s ⏱️

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Python FileSkillsSource discovery logic to follow the agentskills.io directory-structure specification, aligning behavior with the .NET implementation.

Changes:

  • Switch resource discovery to non-recursive scanning of spec-defined folders (references/, assets/) and script discovery to scripts/.
  • Add customizable resource_directories and script_directories options (including "." to scan the skill root).
  • Expand and update test coverage to validate new discovery behavior, ordering, and directory-name validation.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
python/packages/core/agent_framework/_skills.py Implements directory-based, non-recursive resource/script discovery and adds directory configuration + validation utilities.
python/packages/core/tests/core/test_skills.py Updates existing tests and adds new cases for spec-aligned discovery and directory validation behavior.
Comments suppressed due to low confidence (1)

python/packages/core/agent_framework/_skills.py:2545

  • _discover_script_files calls _has_symlink_in_path(resolved_target, root_directory_path) without checking that resolved_target is contained within root_directory_path. With an absolute/escaping directories entry, _has_symlink_in_path raises ValueError and stops discovery. Add a containment guard (or catch ValueError) and skip the directory with a warning to keep discovery robust.
            # Directory-level symlink check for non-root directories
            if not is_root and FileSkillsSource._has_symlink_in_path(resolved_target, root_directory_path):
                logger.warning(
                    "Skipping script directory '%s': symlink detected in path under skill directory '%s'",
                    directory,
                    skill_dir_path,
                )
                continue

Comment thread python/packages/core/agent_framework/_skills.py Outdated
Comment thread python/packages/core/agent_framework/_skills.py Outdated
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated Code Review

Reviewers: 4 | Confidence: 90% | Result: All clear

Reviewed: Correctness, Security Reliability, Test Coverage, Design Approach


Automated review by SergeyMenshykh's agents

- Narrow Windows absolute path check to proper drive-root pattern
  (re.match r'^[A-Za-z]:[/\\]') to avoid rejecting valid POSIX names
- Add _is_path_within_directory guard before _has_symlink_in_path in
  both discovery methods to prevent ValueError on escaped paths

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@SergeyMenshykh SergeyMenshykh marked this pull request as ready for review May 13, 2026 11:42
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated Code Review

Reviewers: 4 | Confidence: 90% | Result: All clear

Reviewed: Correctness, Security Reliability, Test Coverage, Design Approach


Automated review by SergeyMenshykh's agents

Comment thread python/packages/core/agent_framework/_skills.py
Comment thread python/packages/core/agent_framework/_skills.py
Comment thread python/packages/core/agent_framework/_skills.py
Address review comment: _discover_resource_files and _discover_script_files
previously swallowed OSError silently when iterdir() failed. Now log a
warning so permission errors and transient FS failures are visible
instead of making resource/script directories silently disappear.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@SergeyMenshykh SergeyMenshykh changed the title Python: Align file skill folder discovery with agentskills.io spec [BREAKING] Python: Align file skill folder discovery with agentskills.io spec May 14, 2026
@SergeyMenshykh SergeyMenshykh changed the title [BREAKING] Python: Align file skill folder discovery with agentskills.io spec Python: [BREAKING] Align file skill folder discovery with agentskills.io spec May 14, 2026
@SergeyMenshykh SergeyMenshykh changed the title Python: [BREAKING] Align file skill folder discovery with agentskills.io spec [BREAKING] Python: Align file skill folder discovery with agentskills.io spec May 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Python: Align file skill folder discovery with agentskills.io spec

4 participants