#2530

fix: discover all skills in catalog layouts

Openshivenaggarwalwants to merge intofrom

Two separate problems made catalog layouts drop skills without any warning (#2528).

Root causes:

  • The bounded walk inside skill containers reused SKIP_DIRS (node_modules, .git, dist, build, __pycache__), which exists to keep the repo-wide fallback out of build output. A category named build or dist was therefore never entered, and skills/build/<skill>/SKILL.md was skipped whether you pointed at the repo or at ./skills. This is the "order dependence" in the report: renaming build/ to zzbuild-probe/ helped because of the name, not the sort order.
  • skills add ./skills treats the path it is given as a repo root, so it was only walked one level deep. Nested skills were then found only by the recursive fallback, which also skips build/dist (11 of 14 found) and doesn't run at all once a direct child has a SKILL.md (adding skills/aaa-probe/SKILL.md dropped the result to 1).

Fix:

  • Container walks now only skip node_modules, .git and __pycache__. The repo-wide recursive search still skips dist and build as before.
  • A search path that is itself a skills directory (a local path, or a subpath such as owner/repo/tree/main/skills) is walked like any other container: up to three levels deep, stopping below a discovered skill. skills add ./repo and skills add ./repo/skills now return the same skills.
  • The exception is a skills directory that has its own skills/ subdirectory. That is a checkout of a repo named skills, so it keeps repo-root behavior and unrelated files such as examples/demo/SKILL.md stay hidden.
  • findSkillMdPaths (blob fast path) mirrors all of this, as its comment asks, so blob and clone installs agree.
  • One sentence in docs/creating-skills.md (Skill Discovery) noting that a skills directory passed directly is walked as a container.

Testing:

  • New tests/catalog-discovery.test.ts covers discoverSkills and findSkillMdPaths:
    • build/dist categories, with node_modules still skipped
    • a skills directory passed directly or as a subpath, including the three-level bound
    • a direct-child skill next to nested ones
    • a repo checkout named skills keeping repo-root behavior
  • The five tests for the reported bugs fail on main. The two checkout tests pass on main and guard against walking such a checkout as a container. All seven pass with this change.
  • With the reporter's 14-skill layout, skills add <path> --list now finds 14 skills from both ./repo and ./repo/skills (was 11 and 11), and 15 with the extra aaa-probe skill (was 1).
  • pnpm test, pnpm type-check, pnpm format:check and pnpm build pass.

Notes:

  • Directories with other names (e.g. ./my-local-skills) keep the current repo-root behavior.
  • Inside a skills directory that is passed directly, skills more than three levels deep now need --full-depth, the same as when the repo root is passed. Before, the fallback sometimes found them, depending on what else was in the directory.

Fixes #2528