-
Notifications
You must be signed in to change notification settings - Fork 227
brought support for running Fara on Windows devices #19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this 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 fixes a critical bug that prevented FARA from running in headful mode on Windows devices. The issue was caused by unconditional attempts to start Xvfb (a Linux virtual framebuffer) which doesn't exist on Windows, resulting in a [WinError 2] crash. The fix adds platform detection to skip Xvfb initialization on Windows while maintaining existing behavior on Linux/macOS.
Key Changes:
- Added
platformmodule import for OS detection - Modified browser initialization to conditionally start Xvfb only on non-Windows systems
- Updated cleanup logic to avoid stopping non-existent Xvfb processes on Windows
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if not self.headless and platform.system() != "Windows": | ||
| self.start_xvfb() |
Copilot
AI
Nov 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider caching the platform check result as a class attribute during __init__ (e.g., self.is_windows = platform.system() == "Windows"). Currently, platform.system() is called in three different methods (_init_regular_browser, _init_persistent_browser, and close), which could be optimized by computing it once. This would also improve code readability by making the Windows detection logic more explicit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot open a new pull request to apply changes based on this feedback
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot open a new pull request to apply changes based on this feedback
|
Wanna play |
|
Is there anything else needed to accept this PR? |
|
Hi @danielkornev , I found there were a few other fixes needed to work on Windows, I made them in PR @danielkornev and merged that. Thank you again for your contribution, I'll be closing this PR and issue as resolved. |
|
Thanks a lot for the quick turnaround! I just tested the update on Windows and everything works great now. Really appreciate the improvements you added on top of the original PR. |
Summary
Running the agent with --headful on Windows caused the browser to crash immediately with:
The root cause was that the code attempted to launch Xvfb—a Linux virtual framebuffer that does not exist on Windows. This resulted in repeated failures to start the browser.
This PR updates the browser initialization logic to only start Xvfb on Unix-like systems and skip it on Windows, where Playwright can run headfully using the native display.
The Problem
_init_regular_browser()and_init_persistent_browser()attempted to start Xvfb unconditionally.[WinError 2]because Xvfb is not present.The Solution
importplatform and OS detection.Changes in This PR
platform.system()_init_regular_browser()to skip Xvfb on Windows_init_persistent_browser()to skip Xvfb on Windowsclose()method to only stop Xvfb on non-Windows systemsTesting
Repro command (now fixed):
Verified on Windows:
Why This Matters
This fix restores support for Windows devices—an important platform for developers using FARA. Since headful mode is essential for debugging agent behavior, this significantly improves the Windows developer experience.