⚡ Bolt: Remove synchronous I/O operations from core logic#102
Conversation
Replaced `fs.existsSync` with asynchronous file operations (`fs.promises.readFile`, `fs.promises.access`, `fs.promises.mkdir`) wrapped in `try/catch` blocks across the `packages/core` package. This change prevents blocking the Node.js event loop during initialization or loading operations, improving overall application concurrency and responsiveness.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Hey @iotserver24! 👋 I'll go through the changes and help you out with an automated review! 🔍 Starting the review now... |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR migrates the core codebase from synchronous filesystem checks ( ChangesAsync filesystem pattern refactoring
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🤖 Powered by Xibe AI • Auto-generated |
There was a problem hiding this comment.
Pull request overview
This PR removes synchronous filesystem existence checks from the core package, replacing them with async fs/promises operations to avoid blocking the Node.js event loop during initialization and memory/permission loading.
Changes:
- Replaced
fs.existsSyncgating with asyncreadFile/access+try/catchpatterns across core modules. - Simplified initialization paths by relying on async ops (e.g., unconditional
mkdir({ recursive: true })) and handlingENOENTvia exceptions. - Updated internal Bolt notes to capture the “avoid sync fs” learning.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/utils/auto-memory.ts | Switches project memory directory detection from existsSync to async access/readdir flow. |
| packages/core/src/permission-store.ts | Removes existsSync check and relies on async readFile with error fallback. |
| packages/core/src/memory.ts | Replaces sync existence checks with async mkdir + readFile guarded by ENOENT handling. |
| packages/core/src/code-graph.ts | Replaces existsSync(tsconfig) with async fs.promises.access gating before project creation. |
| packages/core/src/agent.ts | Removes sync fs import and uses async readFile for the memory.md fallback path. |
| .jules/bolt.md | Documents the “remove synchronous file ops” learning/action item. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const tsConfigPath = path.join(this.workingDir, 'tsconfig.json'); | ||
| if (fs.existsSync(tsConfigPath)) { | ||
| let hasTsConfig = false; | ||
| try { | ||
| await fs.promises.access(tsConfigPath); | ||
| hasTsConfig = true; | ||
| } catch { | ||
| hasTsConfig = false; | ||
| } |
| try { | ||
| const content = await readFile(fallbackMd, 'utf-8'); | ||
| this.autoMemoryMarkdownSection = `\n\n## Project Memory\n\n${content.trim()}`; | ||
| } catch { /* ignore if not exist */ } |
| try { | ||
| await access(autoDir); | ||
| const names = await readdir(autoDir, { withFileTypes: true }); | ||
| const mdFiles = names.filter((d) => d.isFile() && d.name.endsWith('.md')).map((d) => join(autoDir, d.name)); | ||
| for (const fp of mdFiles.slice(0, MAX_AUTO_LOAD_MEMORIES)) { |
💡 What: Replaced synchronous
fs.existsSyncchecks with asynchronous file operations (fs.promises.readFile,fs.promises.access) and structuredtry/catcherror handling across the core package (e.g.,agent.ts,memory.ts,auto-memory.ts,permission-store.ts,code-graph.ts).🎯 Why: Using synchronous file system operations blocks the Node.js event loop, resulting in micro-stutters and decreased responsiveness, particularly during I/O-heavy processes like initialization or loading settings. It also introduces potential Time-of-Check to Time-of-Use (TOCTOU) race conditions.
📊 Impact: Prevents main thread blocking and improves application concurrency.
🔬 Measurement: Run
pnpm testand verify that the application initializes without hanging and the tests pass successfully.PR created automatically by Jules for task 10425669300620653950 started by @iotserver24
Summary by CodeRabbit
Refactor
Documentation