fix(eng): list nested skill assets instead of directory names 🤖🤖🤖

[SEPURI-SAI-KRISHNA]

Description

parseSkillMetadata() in eng/yaml-parser.mjs only recursed into three hard-coded
directory names when listing a skill's bundled assets:

const assetPaths = ['references', 'assets', 'scripts'];
if (fs.statSync(filePath).isDirectory() && assetPaths.includes(file)) {
  arrayOfFiles = getAllFiles(filePath, arrayOfFiles);  // recurse
} else {
  arrayOfFiles.push(relativePath);                     // pushes directories as assets
}

Any other directory fell into the else branch and was pushed as if it were a file.
Three consequences:

1. Nested subdirectories are still skipped. The allowlist added for yaml-parser.mjs doesn't handle skill assets in subdirectories #503 only works
   one level deep, so references/skeletons/, scripts/lib/, and assets/templates/
   are never walked.
2. Directory names are rendered as asset entries. Before this change,
   docs/README.skills.md listed qdrant-scaling as having four assets named
   minimize-latency, scaling-data-volume, scaling-qps, scaling-query-volume —
   those are folders, and the 12 SKILL.md files inside them were invisible.
3. The 5 MB asset size check silently skips files. eng/validate-skills.mjs
   iterates this same list, so it called statSync on directories and never checked
   anything inside a non-allowlisted folder. An oversized file in
   skills/<name>/templates/ passes validation today.

23 skills are affected, hiding roughly 120 bundled files from the generated docs and
website data.

Fix

Recurse unconditionally and never emit a directory. This makes parseSkillMetadata()
match parseHookMetadata() directly below it, which already walks all subdirectories,
and matches eng/materialize-plugins.mjs, which copies skill folders recursively
regardless of directory name.

Verification

• npm start — only docs/README.skills.md changed (regenerated, included here)
• npm run plugin:validate — passes (94 local + 43 external)
• npm run skill:validate — passes, no new size violations
• node --test across all 8 eng/**/*.test.mjs files — 55 tests, 0 failures
• No directory entries remain in any skill's asset list

Added eng/yaml-parser.test.mjs as a regression guard: it fails 1/2 against the
previous implementation and passes 2/2 with the fix.

Notes

Related to #503 — this is a follow-up to a regression that fix introduced, not a
reopening of it.

This fix was developed with AI assistance; I have reviewed and verified the change
locally.

[github-actions]

🔴 Contributor Reputation Check: HIGH risk

Check	Risk
Profile	HIGH
Credential audit	NONE

Maintainers: please review this contributor before merging.
See the workflow run for full details.
Automated check powered by AGT.

[Copilot]

Pull request overview

Fixes skill asset discovery by recursively listing bundled files.

Changes:

• Recurses through all skill subdirectories.
• Adds regression tests for nested assets.
• Regenerates skill documentation.

Reviewed changes

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

File	Description
eng/yaml-parser.mjs	Recursively discovers skill assets.
eng/yaml-parser.test.mjs	Tests nested asset discovery.
docs/README.skills.md	Lists previously hidden asset files.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[SEPURI-SAI-KRISHNA]

Good catch, fixed. Temp paths are now tracked and removed in an after hook, matching the pattern in eng/external-plugin-quality-gates.test.mjs; verified the run
leaves no skill-* directories behind. Also added a third test covering nested SKILL.md files, since the recursion change means only the skill's own root SKILL.md is
excluded from the asset list.

[Copilot]

Pull request overview

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

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

eng/yaml-parser.mjs:152

• statSync() follows directory symlinks, so unconditional recursion can now leave the skill root (for example, a tracked templates -> ../other-skill link) or enter a cycle and make parseSkillMetadata() fail. That causes the asset list and 5 MB validation to inspect files that are not actually contained in this skill. Please use directory-entry/lstat information and explicitly reject or avoid recursing through symlinks, with a regression test for the chosen policy. The equivalent hook traversal at eng/yaml-parser.mjs:226 should follow the same policy.

          if (fs.statSync(filePath).isDirectory()) {
            arrayOfFiles = getAllFiles(filePath, arrayOfFiles);

[Copilot]

Pull request overview

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

[SEPURI-SAI-KRISHNA]

Fair point, and you're right that this PR widened the exposure — before the change a symlinked non-spec directory was never traversed, so unconditional recursion made
escaping the skill root newly reachable. Both traversals now use readdirSync(..., { withFileTypes: true }) and skip symlinks outright, so a link can neither leave the folder nor
cycle. Applied the same policy to the hook traversal as suggested. Added a regression test covering both shapes — a link pointing outside the skill and one pointing back at its root;
it fails without the guard. npm start output is unchanged, since the repo has no symlinks today.

[SEPURI-SAI-KRISHNA]

Good catch on the file case — fixed. I confirmed copyFileSync dereferences a symlinked file, so it really does ship as a
real file in the materialized plugin, and omitting it from the asset list recreated exactly the metadata/reality gap this PR is fixing. The policy is now narrower: directory symlinks
are skipped (following one can escape the folder), broken links are skipped, and file symlinks are listed. Tests cover all three.

On the directory case, I checked and copyDirRecursive doesn't silently ship it — entry.isDirectory() is false for a symlink, so copyFileSync throws EISDIR and
materialization fails loudly. I've left the materialization and validate-skills.mjs changes out of this PR: changing what gets copied into plugins, or adding a new validation
error class, is a repo-wide policy decision rather than part of this asset-listing fix, and there are currently no symlinks anywhere under skills/, hooks/, agents/, instructions/,
extensions/ or plugins/. Happy to open a follow-up issue proposing a single explicit symlink policy across all three paths if maintainers want that.

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[SEPURI-SAI-KRISHNA]

Merged main in to clear the conflict in docs/README.skills.md. The conflict was only in that generated file, eng/yaml-parser.mjs has not changed on main since #2161 in July. I resolved it by taking main's version and regenerating with npm start rather than hand-merging, so the committed file is exactly what the build produces.

While re-syncing I noticed a fresh example of the bug this PR fixes, in a skill that landed yesterday. skills/anti-ui-slop/ (added in #2771) bundles an agents/ directory and a reference/ directory, note the singular, alongside a separate references/. Neither agents nor reference is in the ['references', 'assets', 'scripts'] allowlist, so both are pushed into the asset list as bare directory names and everything inside them disappears.

Simulating the pre-fix getAllFiles against that skill:

OLD (8 entries)                       NEW (13 entries)
  CHECKSUMS.sha256                      CHECKSUMS.sha256
  LICENSE                               LICENSE
  MANIFEST.json                         MANIFEST.json
  MODIFICATIONS.md                      MODIFICATIONS.md
  NOTICE                                NOTICE
  agents            <- directory        agents/openai.yaml
  reference         <- directory        reference/audit.md
                                        reference/distill.md
                                        reference/ios.md
                                        reference/new-work.md
                                        reference/operate.md
                                        reference/polish.md
  references/uizze-reference-policy.md  references/uizze-reference-policy.md

Seven real files hidden behind two entries that are not files at all. This also means eng/validate-skills.mjs silently skips those seven when it applies its 5 MB bundle check, since it walks the same list.

That is the failure mode this PR removes, and it will keep recurring for any skill that organises bundled content under a directory name outside the three-entry allowlist.

Checks are green and the diff is unchanged at three files.