← Back to PRs

#12076: fix(skills): recursive directory filtering to actually exclude venv/node_modules

by xiaoyaner0201 open 2026-02-08 20:22 View on GitHub →
channel: discord agents stale
## Summary Fix the skills directory filtering implementation from PR #10204 to actually work. ## Problem The original implementation symlinked entire skill directories: ```ts fs.symlinkSync(sourcePath, targetPath, "dir"); // symlinks whole directory ``` This meant `venv/`, `node_modules/`, `.git/`, etc. **inside** skill directories were still scanned, defeating the purpose of filtering. ## Solution Recursive directory creation with file-only symlinks: ```ts if (entry.isDirectory()) { fs.mkdirSync(targetPath); createFilteredSkillDir(sourcePath, targetPath); // recurse, filter at every level } else { fs.symlinkSync(sourcePath, targetPath, "file"); // symlink files only } ``` ## Changes 1. **Recursive filtering** - Filter `venv`, `node_modules`, `.git`, etc. at every directory level, not just top-level 2. **Export `IGNORED_DIR_PATTERNS`** - Allow tests to import the constant 3. **Remove fallback behavior** - Fail-fast instead of silently falling back to unfiltered scan (per code review) 4. **Fix Vitest 4 compatibility** - Update deprecated test syntax ## Testing All 19 tests pass: ``` Test Files 1 passed (1) Tests 19 passed (19) ``` ## Review This fix was developed with assistance from an AI Council review (Opus/Sonnet/GPT multi-model deliberation) that identified the core issue in the original implementation. Refs: #9921, #10204 <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> This PR changes skill discovery to avoid scanning large dev directories by building a recursively-filtered temporary skills tree and symlinking individual files rather than whole directories. It adds a dedicated `filter-skill-dirs` module (plus tests) and updates workspace skill loading to run through the filtered temp dir. It also adjusts Discord listener handlers to be explicitly fire-and-forget while still measuring slow handlers. <h3>Confidence Score: 4/5</h3> - Mostly safe to merge, but a couple of behavior/diagnostics issues should be addressed first. - Core filtering approach and tests look reasonable, and Discord handler change preserves fire-and-forget semantics. Remaining concerns are (1) mutating returned Skill objects in-place during path remapping, and (2) silently swallowing filesystem errors during filtered tree creation, which can lead to missing skills without any signal. - src/agents/skills/workspace.ts, src/agents/skills/filter-skill-dirs.ts <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs