Skip to content

Conversation

@davidfirst
Copy link
Member

On macOS, FSEvents has a system-wide limit of ~500 streams which causes "Error starting FSEvents stream" when running multiple Bit watchers across workspaces.

This PR:

  • Auto-detects Watchman on macOS and uses it as the Parcel watcher backend instead of FSEvents
  • Switches scope file watcher to polling mode (300ms interval) to reduce FSEvents consumption
  • Enhances the error message to recommend installing Watchman: brew install watchman

When Watchman is installed, it acts as a single daemon for all file watching, avoiding the FSEvents limit entirely.

- Auto-detect Watchman on macOS and use it instead of FSEvents to avoid ~500 stream limit
- Switch scope watcher to polling mode (300ms interval) to reduce FSEvents consumption
- Enhance error message to recommend installing Watchman via 'brew install watchman'
Copilot AI review requested due to automatic review settings December 5, 2025 19:50
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses macOS FSEvents stream limitations that cause failures when running multiple Bit watchers. The solution implements automatic Watchman detection and fallback, along with reduced FSEvents usage.

Key Changes:

  • Auto-detects and uses Watchman as the Parcel watcher backend on macOS when available
  • Switches scope file watcher from FSEvents to polling mode (300ms interval)
  • Improves error messaging to guide users toward installing Watchman

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
scopes/workspace/watcher/watcher.ts Adds Watchman detection and auto-configuration for Parcel watcher with new getParcelWatcherOptions() and isWatchmanAvailable() methods
scopes/workspace/watcher/fsevents-error.ts Enhances error message to recommend Watchman installation via Homebrew
scopes/scope/scope/scope.main.runtime.ts Switches scope file watching to polling mode to reduce FSEvents stream consumption

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings December 5, 2025 21:39
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

return this.watchmanAvailable;
}
try {
execSync('watchman version', { stdio: 'ignore', timeout: 5000 });
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The execSync call should include the shell: false option for security. While the command is hardcoded (not using user input), explicitly disabling shell interpretation is a security best practice to prevent command injection vulnerabilities.

Suggested fix:

execSync('watchman version', { stdio: 'ignore', timeout: 5000, shell: false });
Suggested change
execSync('watchman version', { stdio: 'ignore', timeout: 5000 });
execSync('watchman version', { stdio: 'ignore', timeout: 5000, shell: false });

Copilot uses AI. Check for mistakes.
}

/**
* Check if Watchman is installed and running.
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment states "Check if Watchman is installed and running" but the implementation only checks if the watchman version command executes successfully. This doesn't verify that the Watchman daemon is actually running and ready to watch files.

Consider either:

  1. Updating the comment to accurately reflect what's being checked: "Check if Watchman is installed"
  2. Or enhancing the check to verify the daemon is running (e.g., using watchman get-sockname which requires the daemon to be active)

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings December 8, 2025 14:44
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings December 8, 2025 14:48
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +168 to +169
const result = spawnSync('watchman', ['version'], { stdio: 'ignore', timeout: 5000 });
this.watchmanAvailable = result.status === 0;
Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spawnSync call should include error handling for robustness. While the current logic correctly falls back to false when status !== 0, spawnSync can:

  1. Throw exceptions in rare cases (e.g., invalid options or system resource issues)
  2. Return status: null when the process times out or is terminated by a signal
  3. Set result.error when the command fails to spawn

Without error handling, an exception would leave watchmanAvailable as null, causing repeated execution attempts. Consider wrapping the check in a try-catch and checking for result.error:

private isWatchmanAvailable(): boolean {
  if (this.watchmanAvailable !== null) {
    return this.watchmanAvailable;
  }
  try {
    const result = spawnSync('watchman', ['version'], { stdio: 'ignore', timeout: 5000 });
    if (result.error) {
      this.logger.debug(`Watchman check failed: ${result.error.message}`);
      this.watchmanAvailable = false;
    } else {
      this.watchmanAvailable = result.status === 0;
    }
  } catch (err) {
    this.logger.debug(`Error checking Watchman availability: ${err}`);
    this.watchmanAvailable = false;
  }
  return this.watchmanAvailable;
}
Suggested change
const result = spawnSync('watchman', ['version'], { stdio: 'ignore', timeout: 5000 });
this.watchmanAvailable = result.status === 0;
try {
const result = spawnSync('watchman', ['version'], { stdio: 'ignore', timeout: 5000 });
if (result.error) {
this.logger.debug(`Watchman check failed: ${result.error.message}`);
this.watchmanAvailable = false;
} else {
this.watchmanAvailable = result.status === 0;
}
} catch (err: any) {
this.logger.debug(`Error checking Watchman availability: ${err}`);
this.watchmanAvailable = false;
}

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants