Skip to content

fix(discovery): count symlinked skill and plugin directories - #205

Open
rudycelekli wants to merge 1 commit into
uber:mainfrom
rudycelekli:fix/discovery-symlinked-skill-dirs
Open

rudycelekli wants to merge 1 commit into
uber:mainfrom
rudycelekli:fix/discovery-symlinked-skill-dirs

Conversation

@rudycelekli

@rudycelekli rudycelekli commented Oct 19, 2026 •

Copy link
Copy Markdown

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Related issue: N/A (found by reading the code; reproduced on main at 6b33dff)

What changed?
Accept entry.is_symlink as a candidate member too; the existing _manifest_of check drops links to files and dangling links.

Why?
Extractor/surfaces.py:86-87 _is_member requires entry.is_dir for nested surfaces, but gate.list_dir sets is_dir with follow_symlinks=False. A skill linked into place (~/.claude/skills/review -> ~/src/review) is never a declaration, so it is missing from the inventory, although gate.hash_tree explicitly supports roots reached through a link (test_folder_hash.test_a_root_reached_through_a_link_hashes_its_target).

How did you test it?

  • Reproduction: uv run --python 3.11 pytest -q adr_discovery/tests_unit/test_m3_surfaces.py -> main: 1 failed (only "other" found), 8 passed; branch: 9 passed. Pipeline probe on branch: linked skill becomes a skill asset at ~/.claude/skills/review with skill_sha256.
  • Test suite: Discovery pytest -q 561 passed; ruff check . clean.
  • Not tested: symlinked plugin dirs on a live host (same code path, covered only by reasoning).

Potential risks
The change only affects the input described above; other paths go through the same code as before, and the component's existing tests pass.

Directory listings report is_dir without following links, so a skill
linked into place (`ln -s ~/src/review ~/.claude/skills/review`) failed
the membership test and never became a declaration, although the host
loads it and gate.hash_tree already hashes a root reached through a link.

Accept symlinks as candidate members; the existing manifest check still
drops links to files and dangling links.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants