fix(skills): follow directory symlinks in SKILL.md discovery - #44129
Open
Helmi wants to merge 3 commits into
Open
fix(skills): follow directory symlinks in SKILL.md discovery#44129Helmi wants to merge 3 commits into
Helmi wants to merge 3 commits into
Conversation
alt-glitch
added
type/bug
alt-glitch
added
comp/gateway
teknium1
reviewed
Jul 14, 2026
teknium1
left a comment
teknium1
left a comment
Contributor
There was a problem hiding this comment.
Thanks for consolidating discovery around the existing symlink-aware iterator. The premise still holds on current main: hermes_cli/profiles.py:789 uses rglob("SKILL.md"), while agent/skill_utils.py:796 uses os.walk(..., followlinks=True).
Problems
- The new tests call
symlink_to()without an unavailable-symlink skip (tests/agent/test_skill_utils.py:214;tests/hermes_cli/test_profiles.py:1379). Existing skill tests skipOSError/NotImplementedErrorfor this condition attests/agent/test_skill_commands.py:41-50. - Current main has additional discovery paths not covered by this patch:
tools/skills_sync.py:84,tools/skill_usage.py:858, and the desktop learning graph atagent/learning_graph.py:80(introduced by96552c31e3). Those paths would still miss directory-symlinked skills.
Suggested changes
- Make the added symlink tests skip when symlinks are unavailable.
- Reapply the canonical iterator to the remaining current-main discovery paths and add focused coverage for them.
This is an automated hermes-sweeper review.
| real = skills_dir / "real-skill" | ||
| real.mkdir() | ||
| (real / "SKILL.md").write_text("---\nname: real-skill\n---\n", encoding="utf-8") | ||
| (skills_dir / "linked-skill").symlink_to(vault) |
Contributor
There was a problem hiding this comment.
Please handle OSError/NotImplementedError and skip when directory symlinks are unavailable. Existing skill symlink tests use this pattern so Windows CI does not fail before reaching the assertion.
teknium1
added
sweeper:risk-compatibility
Helmi
force-pushed
the
fix/skill-discovery-symlinks
branch
from
July 21, 2026 15:25
5aba92a to
1573a0b
Compare
The runtime skill loader walks skills directories with os.walk(followlinks=True) via iter_skill_index_files(), so symlinked skill folders load and work. Every other SKILL.md discovery site used Path.rglob(), which never descends into symlinked directories — so symlinked skills worked at runtime but were invisible to the profile skill count, skill usage tracking, the skills hub, curator backups, profile distribution, dump, the gateway's disabled-skill hint, and the dashboard's skills endpoints. Switch all discovery sites to the existing iter_skill_index_files() helper. This also applies the shared EXCLUDED_SKILL_DIRS pruning uniformly (one site previously scanned without it) and yields deterministic ordering everywhere.
… support Address review feedback: wrap symlink_to in try/except for OSError/NotImplementedError and pytest.skip, matching the established pattern in test_image_source, test_skill_manager_tool, and test_transcription_tools so Windows CI does not fail before reaching the assertion.
The symlink-aware iterator was applied to most discovery call sites, but three current-main paths still used a raw rglob and therefore could not see directory-symlinked skills: - tools/skills_sync.py — the external skill index, so a vault-linked skill was invisible to shadow detection and sync_skills wrote a bundled copy over the top of the delegated one - tools/skill_usage.py — external skill lookup by frontmatter name - agent/learning_graph.py — the desktop learning graph's own walk Converts all three to iter_skill_index_files and adds focused coverage for each. The added symlink tests skip when the platform cannot create directory symlinks, matching the existing convention in tests/agent/test_skill_commands.py.
Helmi
force-pushed
the
fix/skill-discovery-symlinks
branch
from
July 31, 2026 07:51
1573a0b to
f0df481
Compare
This was referenced Aug 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I manage some skills with an external tool that symlinks them from a central vault into each agent's skills folder. They load and run fine — the runtime loader walks with
followlinks=True— but everything else sees a different reality: the profile list undercounts skills, usage tracking misses them, curator backups skip them, and the dashboard's skills views don't know they exist. Turns out the loader is the only place that follows symlinks; the other discovery sites all userglob("SKILL.md"), which never descends into symlinked dirs (on any Python version — 3.13'srecurse_symlinksis opt-in).Fix: swapped every discovery site to the
iter_skill_index_files()helper you already have, so there's one canonical answer to "what skills are installed". Nice side effects: the shared exclusion set now applies uniformly (one endpoint scanned without it), and ordering is deterministic everywhere.Tests: added symlink coverage for
iter_skill_index_files()and_count_skills(); the test files for all touched modules pass (448 tests). The full suite shows 45 failures on my machine, but they're identical on a clean checkout of main (auth/systemd/environment-specific) — happy to dig into any of them if they look unexpected on your side.