Skip to content

Python: Stop skill discovery below skill boundaries - #6685

Closed
Lovlace777 wants to merge 2 commits into
microsoft:mainfrom
Lovlace777:lovelace/fix-nested-skill-discovery
Closed

Python: Stop skill discovery below skill boundaries#6685
Lovlace777 wants to merge 2 commits into
microsoft:mainfrom
Lovlace777:lovelace/fix-nested-skill-discovery

Conversation

@Lovlace777

Copy link
Copy Markdown

Motivation & Context

Summary

File-based skill discovery currently keeps descending after it finds a SKILL.md. This can treat content nested beneath a skill boundary as an independent skill root. Stop discovery at the first skill boundary so everything below it remains part of that skill.

Description & Review Guide

  • What are the major changes? Return immediately after recording a directory that contains SKILL.md, and add a regression test with a nested SKILL.md.
  • What is the impact of these changes? Standard flat skill layouts are unchanged, while nested skill content is no longer discovered as a separate skill.
  • What do you want reviewers to focus on? The boundary behavior in _discover_skill_directories and the regression test structure.

Validation

  • uv run pytest packages/core/tests/core/test_skills.py::TestDiscoverSkillDirectories -q — 7 passed.
  • uv run pytest packages/core/tests/core/test_skills.py -q — passed.
  • uv run poe test -P core — 3350 passed, 30 skipped, 2 xfailed.
  • uv run poe syntax -P core -C — passed.
  • uv run pyright packages/core/agent_framework/_skills.py — 0 errors.
  • uv run pyright -p pyrightconfig.tests.json packages/core/tests/core/test_skills.py — 0 errors.
  • git diff --check — passed.

Notes

No public API, dependency, or compatibility changes. The full core Pyright sweep could not resolve locally absent optional Graphviz, OpenTelemetry exporter, Azure Agent Server, and Azure Content Understanding dependencies; focused strict checks for both changed files pass.

Related Issue

Fixes #6682

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and the title prefix in sync automatically.

Copilot AI review requested due to automatic review settings June 23, 2026 15:14
@moonbox3 moonbox3 added the python Usage: [Issues, PRs], Target: Python label Jun 23, 2026
@Lovlace777
Lovlace777 marked this pull request as ready for review June 23, 2026 15:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Stops file-based skill discovery from recursing into subdirectories once a skill boundary (SKILL.md) is found, ensuring nested content remains part of the parent skill rather than being treated as separate skill roots.

Changes:

  • Update FileSkillsSource._discover_skill_directories to return immediately after recording a directory containing SKILL.md.
  • Add a regression test covering a nested SKILL.md under a parent skill boundary.

Reviewed changes

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

File Description
python/packages/core/agent_framework/_skills.py Adds an early return in directory traversal to stop descending below a directory containing SKILL.md.
python/packages/core/tests/core/test_skills.py Adds a regression test asserting nested skill roots are not discovered beneath a parent SKILL.md boundary.

@github-actions

Copy link
Copy Markdown
Contributor

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
packages/core/agent_framework
   _skills.py10223796%294, 541, 1007, 1022, 1024–1025, 1381–1382, 1394–1395, 1625, 1654, 2117, 2573–2574, 2709, 2714, 2717, 2722, 2749, 2754, 2808, 2817, 2822, 2825, 2830, 2854, 2859, 3102–3103, 3452, 3679–3680, 3707–3708, 3715–3716
TOTAL40697462688% 

Python Unit Test Overview

Tests Skipped Failures Errors Time
8119 34 💤 0 ❌ 0 🔥 2m 12s ⏱️

@SergeyMenshykh

Copy link
Copy Markdown
Contributor

@Lovlace777 please answer the CLA requirement described above to proceed with the PR.

@itxaiohanglover

Copy link
Copy Markdown

Clean fix! Returning early after finding a SKILL.md prevents descending into nested skill directories. The test with parent-skill containing nested-skill is the perfect regression case.

@giles17

giles17 commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

@Lovlace777 reminder to answer the CLA requirement so we can merge the PR

@giles17

giles17 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Closing as fix is no longer valid

@giles17 giles17 closed this Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: Skill directory search should stop recursing after finding SKILL.md

8 participants