fix: workspace already opened#6721
Conversation
naman-bruno
requested review from
bijin-bruno,
helloanoop and
lohit-bruno
as code owners
WalkthroughThe workspace watcher configuration is refined to suppress initial file events by enabling Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![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
🤖 Fix all issues with AI agents
In @packages/bruno-electron/src/app/workspace-watcher.js:
- Around line 159-163: The workspace watcher only listens for 'change' events
but recreation of workspace.yml emits an 'add' event, so update the watcher to
listen for both 'add' and 'change' and correct the comment: add an 'add'
listener that calls handleWorkspaceFileChange(win, workspacePath) alongside the
existing 'change' listener (refer to watcher.on('add', ...) and
watcher.on('change', ...) and the handleWorkspaceFileChange function) and revise
the comment to state that both 'add' and 'change' are required to detect file
recreations and modifications.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bruno-electron/src/app/workspace-watcher.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-electron/src/app/workspace-watcher.js
🧠 Learnings (1)
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-electron/src/app/workspace-watcher.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Unit Tests
- GitHub Check: SSL Tests - macOS
- GitHub Check: Playwright E2E Tests
- GitHub Check: SSL Tests - Windows
- GitHub Check: CLI Tests
- GitHub Check: SSL Tests - Linux
🔇 Additional comments (1)
packages/bruno-electron/src/app/workspace-watcher.js (1)
150-150: Good fix for preventing duplicate workspace loading.Setting
ignoreInitial: truecorrectly prevents the watcher from firing events for the existing workspace.yml file when the watcher starts, which would have caused duplicate workspace initialization.
| // Only listen for 'change' events - 'add' event is not needed because: | ||
| // 1. The workspace is already loaded when the watcher is started | ||
| // 2. ignoreInitial: true prevents firing for existing files | ||
| // 3. If workspace.yml is deleted and recreated, 'change' will catch it | ||
| watcher.on('change', () => handleWorkspaceFileChange(win, workspacePath)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Test chokidar behavior for file recreation with ignoreInitial: true
# First check if chokidar is available and create test script
cd /tmp && cat > test-chokidar.js << 'EOF'
const chokidar = require('chokidar');
const fs = require('fs');
const path = require('path');
const testFile = path.join(__dirname, 'test-workspace.yml');
// Clean up any existing test file
try {
fs.unlinkSync(testFile);
} catch (e) {}
// Create initial file
fs.writeFileSync(testFile, 'test: initial');
const watcher = chokidar.watch(testFile, {
ignoreInitial: true,
persistent: true
});
const events = [];
watcher.on('add', () => {
events.push('add');
console.log('EVENT: add');
});
watcher.on('change', () => {
events.push('change');
console.log('EVENT: change');
});
watcher.on('unlink', () => {
events.push('unlink');
console.log('EVENT: unlink');
});
setTimeout(() => {
console.log('\n=== Deleting file ===');
fs.unlinkSync(testFile);
}, 500);
setTimeout(() => {
console.log('\n=== Recreating file ===');
fs.writeFileSync(testFile, 'test: recreated');
}, 1000);
setTimeout(() => {
console.log('\n=== Results ===');
console.log('Events captured:', JSON.stringify(events));
watcher.close();
try {
fs.unlinkSync(testFile);
} catch (e) {}
process.exit(0);
}, 1500);
EOF
# Install chokidar if not available and run test
npm install --prefix /tmp chokidar > /dev/null 2>&1 && node test-chokidar.jsRepository: usebruno/bruno
Length of output: 181
Update the workspace watcher to handle file recreation and correct the comment.
The test confirms that file recreation fires an 'add' event, not 'change'. The comment on line 162 is inaccurate: "If workspace.yml is deleted and recreated, 'change' will catch it" is false. When the file is recreated, the 'add' event fires, which is not currently being listened to.
To match the pattern used in the environment watcher (lines 178-188), add an 'add' event listener:
watcher.on('add', () => handleWorkspaceFileChange(win, workspacePath));
watcher.on('change', () => handleWorkspaceFileChange(win, workspacePath));Also update the comment to accurately reflect that both 'add' and 'change' events are needed to detect initial changes and recreations.
🤖 Prompt for AI Agents
In @packages/bruno-electron/src/app/workspace-watcher.js around lines 159 - 163,
The workspace watcher only listens for 'change' events but recreation of
workspace.yml emits an 'add' event, so update the watcher to listen for both
'add' and 'change' and correct the comment: add an 'add' listener that calls
handleWorkspaceFileChange(win, workspacePath) alongside the existing 'change'
listener (refer to watcher.on('add', ...) and watcher.on('change', ...) and the
handleWorkspaceFileChange function) and revise the comment to state that both
'add' and 'change' are required to detect file recreations and modifications.
2 participants
Description
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.