feat(local-mount)!: async createMount yields the event loop during init#105
Merged
Conversation
Closes #104. createMount used to walk the project tree synchronously via readdirSync + copyFileSync, freezing the consumer's event loop for the entire init window. A consumer-side setInterval (e.g. an `ora` spinner) would render its first frame and then visibly hang until createMount returned. Convert createMount to `Promise<MountHandle>` and have walkProjectTree yield between directory entries — once at the start of each recursion and once per 64 entries inside a directory to handle large flat trees. The goal is consumer ergonomics, not throughput; #102 still owns the perf push (hardlinks, parallel copy, fewer syscalls). Breaking: callers must `await createMount(...)`. The package is pre-1.0 and the only known consumer is the AgentWorkforce CLI, which already wraps the call in an async function. `launchOnMount` awaits internally so its surface is unchanged. Tests: existing mount/auto-sync/launch suites updated to await; new regression drives a 5ms setInterval counter while createMount processes 5000 files across 50 dirs and asserts the counter advanced — a sync walker would leave it at 0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR converts createMount to an async function that yields to the event loop during directory traversal (setImmediate yields, initial and periodic every 64 entries). Tests, launch integration, README, CHANGELOG, and trajectory records are updated accordingly. ChangesAsync createMount with Event-Loop Yielding
Sequence Diagram(s)sequenceDiagram
participant CLI
participant createMount
participant walkProjectTree
participant EventLoop
CLI->>createMount: await createMount(projectDir, mountDir)
createMount->>walkProjectTree: await walkProjectTree(root)
walkProjectTree->>EventLoop: setImmediate yield (initial + periodic)
EventLoop-->>walkProjectTree: resume traversal
walkProjectTree-->>createMount: traversal complete
createMount-->>CLI: resolve(MountHandle)
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}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/local-mount/src/launch.ts (1)
69-77:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMove
await createMount(...)inside the guarded lifecycle path.Right now
createMountruns before thetry/finalizeflow. If mount creation fails mid-initialization, cleanup is skipped and temporary mount state can be left behind.Suggested refactor
export async function launchOnMount(opts: LaunchOnMountOptions): Promise<LaunchOnMountResult> { - const handle: MountHandle = await createMount(opts.projectDir, opts.mountDir, { - ignoredPatterns: opts.ignoredPatterns ?? [], - readonlyPatterns: opts.readonlyPatterns ?? [], - excludeDirs: opts.excludeDirs ?? [], - agentName: opts.agentName, - includeGit: opts.includeGit, - }); + let handle: MountHandle | undefined; let syncedCount = 0; let finalized = false; let autoSync: AutoSyncHandle | undefined; const finalize = async (): Promise<void> => { - if (finalized) return; + if (finalized || !handle) return; finalized = true; try { let autoSyncChanges = 0; if (autoSync) { await autoSync.stop({ signal: opts.shutdownSignal }); @@ - const finalSynced = await handle.syncBack({ signal: opts.shutdownSignal }); + const finalSynced = await handle.syncBack({ signal: opts.shutdownSignal }); syncedCount = autoSyncChanges + finalSynced; if (opts.onAfterSync) { await opts.onAfterSync(syncedCount); } } finally { handle.cleanup(); } }; try { + handle = await createMount(opts.projectDir, opts.mountDir, { + ignoredPatterns: opts.ignoredPatterns ?? [], + readonlyPatterns: opts.readonlyPatterns ?? [], + excludeDirs: opts.excludeDirs ?? [], + agentName: opts.agentName, + includeGit: opts.includeGit, + }); + if (opts.onBeforeLaunch) { await opts.onBeforeLaunch(handle.mountDir); }🤖 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/launch.ts` around lines 69 - 77, The call to createMount is executed before entering the guarded lifecycle, so failures can skip cleanup; move the await createMount(...) into the protected try/finalize flow inside launchOnMount so any error during mount initialization triggers the existing cleanup/finalize logic. Specifically, open the try block in launchOnMount and perform const handle: MountHandle = await createMount(...) there (using the same options object), ensuring the finalize/cleanup path still runs if createMount throws; keep all existing references to handle, finalize, and MountHandle intact.
🤖 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/README.md`:
- Line 30: The README wording overstates the yield cadence: update the
description for createMount / walker (and MountHandle) to say the walker yields
periodically during traversal (e.g., at directory entries and additionally every
64 entries) rather than “between directory entries”; change the phrase “between
directory entries” to something like “periodically during traversal (yields at
entries and every 64 entries)” so it matches the actual implementation.
---
Outside diff comments:
In `@packages/local-mount/src/launch.ts`:
- Around line 69-77: The call to createMount is executed before entering the
guarded lifecycle, so failures can skip cleanup; move the await createMount(...)
into the protected try/finalize flow inside launchOnMount so any error during
mount initialization triggers the existing cleanup/finalize logic. Specifically,
open the try block in launchOnMount and perform const handle: MountHandle =
await createMount(...) there (using the same options object), ensuring the
finalize/cleanup path still runs if createMount throws; keep all existing
references to handle, finalize, and MountHandle intact.
🪄 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: 824c3c1a-6cc3-4a5a-a16b-8b2f140a3fe2
📒 Files selected for processing (10)
.trajectories/completed/2026-05/traj_v1un6n66y38i.json.trajectories/completed/2026-05/traj_v1un6n66y38i.md.trajectories/index.jsonpackages/local-mount/CHANGELOG.mdpackages/local-mount/README.mdpackages/local-mount/src/auto-sync.test.tspackages/local-mount/src/launch.test.tspackages/local-mount/src/launch.tspackages/local-mount/src/mount.test.tspackages/local-mount/src/mount.ts
| } | ||
| ``` | ||
|
|
||
| `createMount` returns `Promise<MountHandle>`. The walker yields the event loop between directory entries so consumer-side timers (e.g. an `ora` spinner driven by `setInterval`) keep firing while the mount is being built. |
There was a problem hiding this comment.
Clarify the yield cadence wording to match implementation.
The text says the walker yields “between directory entries,” but the implementation yields at directory entry plus every 64 entries. Consider wording this as “periodically during traversal” to avoid overpromising per-entry yielding.
🤖 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/README.md` at line 30, The README wording overstates the
yield cadence: update the description for createMount / walker (and MountHandle)
to say the walker yields periodically during traversal (e.g., at directory
entries and additionally every 64 entries) rather than “between directory
entries”; change the phrase “between directory entries” to something like
“periodically during traversal (yields at entries and every 64 entries)” so it
matches the actual implementation.
Resolved conflicts in packages/local-mount/src/mount.ts and packages/local-mount/src/mount.test.ts: - mount.ts walkProjectTree: keep `await` on the recursive call from this branch; adopt main's expanded signature (`currentMountDir`, `excludeRules`, `safeMountDir`) from #102 so the perf changes and the event-loop yields coexist. - mount.test.ts: keep main's broader test name "common cache and build output paths" but mark it `async` and `await createMount`. Convert the two new tests main added (`can opt out of broad default excludes…`, `syncBack: skips files under default excluded paths`) to await createMount as well. vitest: 33 passed (4 files), including the #104 event-loop yield regression and the #102 default-excludes coverage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…y block createMount has side effects before it can throw — it mkdirs the mount directory, writes the marker file, then walks the project tree with copyFileSync calls that can fail (permission, ENOSPC, partial reads). With the call sitting before the try/catch+finalize block, a failure mid-init left a partial mount on disk and skipped the finalize lifecycle. Hoist `let handle: MountHandle | undefined` above finalize and move the createMount call to the top of the try block so any error during mount initialization triggers the existing finalize path. Guard finalize against an undefined handle (createMount itself does not currently self-clean its partial mount, so finalize harmlessly returns when there's no handle to sync or cleanup) and capture handle.mountDir as a local const for the spawn closure to satisfy TypeScript's narrowing across the closure boundary. All 33 vitest cases still pass; typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 11, 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
createMountis nowasyncand returnsPromise<MountHandle>. The walker yields the event loop between directory entries (and every 64 entries inside a single directory) so a consumer'ssetInterval— e.g. anoraspinner — keeps firing while the mount is being built.walkProjectTreerecursed withreaddirSync+copyFileSyncand never yielded, so consumers saw a static spinner frame for the entire init window.Breaking change
Callers must
await createMount(...). There is nocreateMountSync. The package is pre-1.0; the only known external consumer is the AgentWorkforce CLI, which already invokes the function from an async context.launchOnMountawaits internally, so its surface is unchanged.Implementation note
Yields use
await new Promise<void>((resolve) => setImmediate(resolve))— the same pattern already used insidesyncBackfor abort-cooperative scans. One yield at each recursion entry plus one every 64 file entries within a single directory keeps both deeply-nested and very-flat trees responsive without measurable perf cost (the issue calls one-yield-per-directory ""enough"").Test plan
npm testinpackages/local-mount— 31 passed (4 files).npm run typecheckinpackages/local-mount— clean.createMount > yields the event loop during init so consumer setInterval can firedrives a 5mssetIntervalcounter whilecreateMountprocesses 5000 files across 50 directories and asserts the counter advanced ≥ 2 ticks. A sync walker would leave it at 0.🤖 Generated with Claude Code