feat(local-mount): exclude .npm-cache from initial walk by default#101
Merged
Conversation
Project-local npm caches (created when `npm`/`npx` run with `npm_config_cache=./.npm-cache` or via tools like prpm/skill.sh that keep their cache out of `~/.npm`) routinely accumulate tens of thousands of files and hundreds of megabytes — none of which belong inside an agent's mount. On a real workforce repo this directory alone added ~5s of `copyFileSync` overhead to mount creation per launch. `DEFAULT_EXCLUDED_DIRS` now lists `.npm-cache` alongside `.git` and `node_modules`. The auto-sync watcher's existing `candidates` list already recognized common cache trees (`dist`, `build`, `.next`, `.cache`) as ones to skip when they're excluded — `.npm-cache` fits the same shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe local-mount package now excludes the Changesnpm-cache Default Exclusion
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Devin Review found 1 potential issue.
⚠️ 1 issue in files not directly in the diff
⚠️ .npm-cache missing from buildIgnoreGlobs candidates, so @parcel/watcher still recurses into it (packages/local-mount/src/auto-sync.ts:241)
.npm-cache was added to DEFAULT_EXCLUDED_DIRS in mount.ts:60, but the buildIgnoreGlobs function in auto-sync.ts:241 has a separate hardcoded candidates list that was not updated. This means @parcel/watcher will still subscribe to and recurse into .npm-cache directories, generating events for every file inside them. Those events are filtered by isSyncCandidate, so correctness is preserved, but the entire performance benefit of excluding .npm-cache from the watcher is lost — the PR's own changelog states these directories contain "tens of thousands of files (often hundreds of MB)."
View 2 additional findings in Devin Review.
`buildIgnoreGlobs` keeps its own `candidates` list separate from `DEFAULT_EXCLUDED_DIRS` so callers can extend exclusions via `excludeDirs` without forcing introspection of the underlying Set. The previous commit added `.npm-cache` to the defaults but missed this list, so `@parcel/watcher` still subscribed to every file underneath. Correctness was preserved by `isSyncCandidate`, but the events were being produced and filtered, undoing most of the perf benefit on real caches with tens of thousands of files. Add `.npm-cache` to the watcher candidates and document the contract: anything in `DEFAULT_EXCLUDED_DIRS` MUST appear in this list too. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/local-mount/src/auto-sync.ts (1)
241-246: ⚡ Quick winUse one source of truth for default excluded dirs.
This fixes
.npm-cache, but Line 241–246 still leaves room for future drift. PullDEFAULT_EXCLUDED_DIRSfrom a shared export and buildcandidatesfrom it (or move both to a common constants module if importing frommount.tscreates a cycle).♻️ Proposed refactor
+import { DEFAULT_EXCLUDED_DIRS } from './mount'; // or from shared constants module function buildIgnoreGlobs(ctx: AutoSyncContext): string[] { const globs: string[] = []; - const candidates = ['.git', 'node_modules', '.npm-cache', 'dist', 'build', '.next', '.cache']; + const candidates = [...DEFAULT_EXCLUDED_DIRS, 'dist', 'build', '.next', '.cache']; for (const name of candidates) { if (ctx.isExcluded(name)) { globs.push(`**/${name}`, `**/${name}/**`); } } return globs; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/local-mount/src/auto-sync.ts` around lines 241 - 246, The hard-coded candidates array in auto-sync.ts can drift from the shared DEFAULT_EXCLUDED_DIRS; replace the literal list by importing DEFAULT_EXCLUDED_DIRS (or moving both into a shared constants module if import would create a cycle) and build const candidates = Array.from(DEFAULT_EXCLUDED_DIRS) (or derive it as needed) so auto-sync.ts uses the single source of truth; update any references to candidates in functions like the initial walk/watcher setup to use the derived array.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/local-mount/src/auto-sync.ts`:
- Around line 241-246: The hard-coded candidates array in auto-sync.ts can drift
from the shared DEFAULT_EXCLUDED_DIRS; replace the literal list by importing
DEFAULT_EXCLUDED_DIRS (or moving both into a shared constants module if import
would create a cycle) and build const candidates =
Array.from(DEFAULT_EXCLUDED_DIRS) (or derive it as needed) so auto-sync.ts uses
the single source of truth; update any references to candidates in functions
like the initial walk/watcher setup to use the derived array.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bbe6d393-666d-419d-9355-cc62f22ee086
📒 Files selected for processing (2)
packages/local-mount/CHANGELOG.mdpackages/local-mount/src/auto-sync.ts
✅ Files skipped from review due to trivial changes (1)
- packages/local-mount/CHANGELOG.md
The previous fix added .npm-cache to a hardcoded `candidates` list in buildIgnoreGlobs, with a doc-block telling future contributors that DEFAULT_EXCLUDED_DIRS additions MUST also be mirrored there. That kind of comment-enforced invariant is exactly what eventually drifts. Replace the doc-block contract with a structural one: AutoSyncContext now carries `excludedNames` — the spread of the same Set that drives `isExcluded` — and `buildIgnoreGlobs` iterates that directly. The hardcoded probe list goes away entirely, and the two call sites can no longer disagree because there is only one list. This also fixes a latent bug: user-supplied `excludeDirs` entries (e.g. `excludeDirs: ['my-custom-cache']`) are now hinted to @parcel/watcher in addition to being skipped during the initial walk. Previously they were skipped from the walk but still subscribed to, producing events the in-handler filter had to throw away. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/local-mount/src/auto-sync.ts`:
- Around line 249-251: The watcher ignore globs currently prefix every
ctx.excludedNames entry with "**/" which over-matches path-based excludes
compared to isExcludedPath; update the loop that builds globs so it checks each
name: if the exclude contains a slash (e.g., name.includes('/')) treat it as a
path-based exclude and add root-anchored patterns (name and name + "/**") or
skip the "**/" prefix, otherwise continue to add the "**/" and "**/.../**"
variants for simple directory names; update the code that builds globs in
auto-sync.ts (the loop referencing ctx.excludedNames) to implement this
conditional handling so watcher globs align with isExcludedPath semantics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f9924b14-4f58-47d0-946b-e21be3ba9c26
📒 Files selected for processing (3)
packages/local-mount/CHANGELOG.mdpackages/local-mount/src/auto-sync.tspackages/local-mount/src/mount.ts
✅ Files skipped from review due to trivial changes (1)
- packages/local-mount/CHANGELOG.md
Switching `buildIgnoreGlobs` to iterate the live `excludeSet` exposed
a semantic gap that was unreachable when the probe list was
hardcoded-bare-names: path-style entries like `build/cache` mean
"only the root-anchored prefix" in `isExcludedPath`, but the loop
was unconditionally prefixing `**/`, producing globs that swallow
deep matches like `src/build/cache/foo.txt`. The watcher would drop
those events even though the in-handler filter would have synced
them — a real correctness regression for callers whose excludeDirs
include slashes.
Make `buildIgnoreGlobs` per-root and emit the shape that matches the
canonical predicate:
- bare names → `**/<name>` + `**/<name>/**` (any depth)
- path-style → `<watchRoot>/<name>` (literal absolute → ignorePaths)
+ `<watchRoot>/<name>/**` (anchored descendant glob)
Library defaults (`.git`, `node_modules`, `.npm-cache`) are all bare
names, so released behavior is unchanged. The fix is required because
the API allows path-style entries and `isExcludedPath` honors them,
so the watcher hint must agree.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.npm-cachetoDEFAULT_EXCLUDED_DIRSso the initial mount walk skips it the same way it already skips.gitandnode_modules.npm/npxrun withnpm_config_cache=./.npm-cache, or via tooling likeprpm/skill.shthat keep their cache out of~/.npm) routinely accumulate tens of thousands of files and hundreds of megabytes — none of which belong inside an agent's mount.agentworkforce/workforce), this directory alone held 14,745 files / 399 MB and added ~5s ofcopyFileSyncoverhead to every mount creation. Combined with other in-repo caches, the user-visible "starting sandbox mount → installing skills" gap was 30–60s, all of which is wasted work.Why this is the right default
The
excludeDirsoption is purely additive — there is no API to opt back in. So adding to the defaults is only safe for directories that are unambiguously cache, never source-of-truth..npm-cache:.gitignores);auto-sync.tslists.git,node_modules,dist,build,.next,.cacheas "heavy" dirs the watcher avoids recursing into when they're excluded..npm-cachefits the same shape.Callers who specifically need an npm cache visible inside the mount can point npm at a different location (e.g.
npm_config_cache=$HOME/.npm— the conventional default) or populate one post-mount fromonBeforeLaunch.Test plan
npm testinpackages/local-mount— 30/30 pass, including the updatedexcludes .git, node_modules, and .npm-cache by defaultregression test.npm run typecheckinpackages/local-mount— clean.@agentworkforce/climount creation is faster on a repo with.npm-cachepopulated (afternpm publishof the next release).🤖 Generated with Claude Code