#12076: fix(skills): recursive directory filtering to actually exclude venv/node_modules
channel: discord
agents
stale
Cluster:
Skill Enhancements and Fixes
## 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
#10016: fix: prevent FD exhaustion from skill watcher scanning artifact trees
by oldeucryptoboi · 2026-02-06
88.4%
#19707: fix(agents): apply per-agent skills filter to all run paths
by mcaxtr · 2026-02-18
83.7%
#14023: fix: filter skills watcher to relevant file types to prevent FD exh...
by funmerlin · 2026-02-11
83.3%
#6777: fix(skills): ignore Python venvs and caches in skills watcher
by kirkluokun · 2026-02-02
82.4%
#8291: Fix: Add Python virtual environment ignore patterns to skills watcher
by vishaltandale00 · 2026-02-03
82.1%
#11250: fix: expand skills watcher ignore list and improve session repair l...
by zhangzhefang-github · 2026-02-07
81.7%
#23749: fix some issues
by tronpis · 2026-02-22
81.1%
#16792: fix(skills): use subsystem logger instead of console.log for debug ...
by Limitless2023 · 2026-02-15
80.5%
#9221: fix(skills): use skillKey for env config lookup in snapshots
by gavinbmoore · 2026-02-05
80.1%
#23183: fix(skill-creator): exclude .git and VCS internals from .skill arch...
by aldoeliacim · 2026-02-22
79.8%