Python: [BREAKING] Refactor FileSkillsSource for depth-based discovery and predicate filters

[giles17]

Motivation and Context

Ports the refactoring from .NET PR #6109 to the Python side. The old directory-whitelist approach (resource_directories/script_directories) was rigid — users had to know exact subdirectory names upfront. The new depth-based scanning with optional filter predicates is strictly more flexible and aligns both language implementations.

Description

Refactors FileSkillsSource to replace preconfigured directory whitelists with depth-controlled recursive scanning and user-supplied filter predicates.

Removed:

• resource_directories / script_directories constructor options

Added:

• search_depth option (>= 1, default 2): controls how deep the recursive scan goes within each skill directory. Depth 1 = root only; depth 2 = root + one level of subdirectories.
• script_filter / resource_filter predicate options that receive a FileSkillFilterContext (skill_name + relative_file_path), allowing whitelist/blacklist filtering by file path.
• FileSkillFilterContext class exported from agent_framework.

Preserved:

• All security checks (path containment, symlink detection) using the skill root as the trusted boundary.
• Extension-based filtering (resource_extensions / script_extensions).

Updated:

• SkillsProvider.from_paths() accepts the new parameters.
• Discovery methods refactored from @staticmethod to instance methods.
• Tests rewritten to validate depth-based behavior and filter predicates.

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? If yes, add [BREAKING] prefix to the title of the PR.

[github-actions]

Python Test Coverage Report •

File	Stmts	Miss	Cover	Missing
packages/core/agent_framework
_skills.py	1025	40	96%	294, 541, 1007, 1022, 1024–1025, 1381–1382, 1394–1395, 1676, 1705, 2174, 2624–2625, 2722, 2730, 2735, 2738, 2743, 2763, 2775, 2780, 2878, 2886, 2891, 2894, 2899, 2919, 2928, 2933, 3196–3197, 3546, 3773–3774, 3801–3802, 3809–3810
_tools.py	1138	90	92%	222–223, 400, 402, 415, 440–442, 450, 468, 482, 489, 496, 519, 521, 528, 536, 665, 699–701, 704–706, 708, 714, 765–767, 792, 818, 822, 860–862, 866, 888, 1031–1032, 1036, 1072, 1084, 1091–1094, 1115, 1119, 1123, 1137–1139, 1489, 1581, 1609, 1631, 1639, 1730, 1737–1738, 1797, 1801, 1847, 1908–1909, 1956, 1970, 1973, 1986, 1989, 2012, 2019, 2029, 2033, 2112, 2165, 2185, 2187, 2243, 2322, 2519, 2585–2586, 2738, 2743, 2745–2746, 2815, 2820, 2827
TOTAL	39144	4476	88%

Python Unit Test Overview

Tests	Skipped	Failures	Errors	Time
7815	34 💤	0 ❌	0 🔥	2m 5s ⏱️

[Copilot]

Pull request overview

Refactors Python file-based skill discovery to match the newer .NET behavior by replacing fixed subdirectory whitelists with depth-based recursive scanning and optional predicate filters, while keeping existing containment/symlink safety checks.

Changes:

• Replace resource_directories / script_directories with search_depth (default 2) for recursive discovery within each skill directory.
• Add script_filter / resource_filter predicates using the new exported FileSkillFilterContext to allow per-file inclusion/exclusion decisions.
• Rewrite/extend unit tests to validate depth behavior, filtering, and existing safety guarantees (path traversal + symlink skipping).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
python/packages/core/tests/core/test_skills.py	Updates tests to validate depth-based discovery and filter predicate behavior.
python/packages/core/agent_framework/_tools.py	Minor refactor in approval-request state handling (no functional change intended).
python/packages/core/agent_framework/_skills.py	Implements search_depth + predicate filters for FileSkillsSource, adds FileSkillFilterContext, updates SkillsProvider.from_paths() surface.
python/packages/core/agent_framework/init.py	Exports FileSkillFilterContext from the public package surface.

[github-actions]

Automated Code Review

Reviewers: 5 | Confidence: 89%

✓ Correctness

The refactoring from directory-whitelist to depth-based scanning with filter predicates is well-implemented and correct. The depth logic correctly maps search_depth=1 to root-only, search_depth=2 to root+one-level, etc. Security checks (path containment, symlink detection) are preserved at all recursion levels. Filter predicates are applied after security checks, and the FileSkillFilterContext validation is sound. The from_paths factory method correctly forwards new parameters. No correctness bugs found.

✓ Security Reliability

The refactoring correctly preserves all security invariants (path containment, symlink detection at both directory and file levels). The depth-based recursion applies directory-level security checks before descending and file-level checks before including any resource. One minor reliability concern: unhandled exceptions from user-provided filter predicates would crash the entire skill discovery process rather than gracefully skipping the affected file.

✓ Test Coverage

Test coverage for the refactored FileSkillsSource is generally strong. Depth-based discovery is well-tested for resources at both unit and integration levels. Filter predicates are tested for both resources and scripts. However, there's an asymmetry: resource_filter has a dedicated context-verification test (test_resource_filter_receives_correct_context) confirming that skill_name and relative_file_path are passed correctly, but script_filter lacks an equivalent test. Similarly, from_paths has a passthrough test for resource_filter and search_depth but not for script_filter. These are low-severity gaps since the code paths are structurally identical.

✓ Failure Modes

The refactoring from directory-whitelists to depth-based recursive scanning is well-implemented from a failure-mode perspective. Error handling for OS-level failures (iterdir OSError), security checks (symlinks, path traversal), and input validation (search_depth >= 1) are all preserved or improved. User-provided filter predicates follow standard Python fail-fast callback semantics. No silent failures, lost errors, or partial-write scenarios were identified.

✗ Design Approach

The refactor mostly hangs together, but the new recursive file discovery no longer preserves skill boundaries when one skill directory contains another nested skill directory. Because nested SKILL.md folders are still discovered as separate skills elsewhere, the current approach can silently attach a child skill’s resources/scripts to its parent as well, which is a design regression worth fixing before merge.

Flagged Issues

• Nested skill directories are treated as separate skills by _discover_skill_directories (lines 3161-3195), but the new recursive resource/script scanners descend into every subdirectory (lines 2796-2807 and 2945-2956) without checking for a nested SKILL.md. With parent/SKILL.md and parent/child/SKILL.md, the child's files will be surfaced under both skills. The scaners should stop recursing into subdirectories that contain their own SKILL.md.

---

Automated review by giles17's agents

[github-actions]

Flagged issue

Nested skill directories are treated as separate skills by _discover_skill_directories (lines 3161-3195), but the new recursive resource/script scanners descend into every subdirectory (lines 2796-2807 and 2945-2956) without checking for a nested SKILL.md. With parent/SKILL.md and parent/child/SKILL.md, the child's files will be surfaced under both skills. The scaners should stop recursing into subdirectories that contain their own SKILL.md.

---

Source: automated DevFlow PR review