[Refactor] Add cua-driver to replace the os_host module#10
Conversation
Project context file for Codex/Cursor/Qoder covering: - Architecture overview and module responsibilities - Code quality requirements (SOLID, Occam's Razor, industrial-grade) - Design philosophy (signal-driven, Context Pipeline, progressive trust) - Development conventions and tech stack - CLI commands and testing guide
…traints Remove all volatile content (module lists, CLI commands, file paths). Now purely a guiding document for AI assistants covering: - 5 non-negotiable design principles (signal-driven, context pipeline, progressive trust, Occam's razor, LLM-native) - Code quality requirements (SOLID, robustness, English-only) - Architecture principles (Protocol-DI, config-driven, graceful degradation) - Implementation workflow (Protocol → implement → test → integrate) - Testing philosophy (hermetic, compile-first, no-regression) - Anti-patterns to avoid - Naming conventions
Execution layer: CuaDriverClient via MCP stdio to cua-driver - Implements HostRpc Protocol (DIP boundary preserved) - Methods mapping (32 constants → cua-driver MCP tools) - Verify-Then-Escalate (AX background → PX pixel → foreground) - Element token tracking for stale detection - Heartbeat keepalive + exactly-once reconnect - Local dispatch for file/clipboard ops (no cua-driver dependency) Event layer: Pure Python cross-platform observers - FileSystemObserver (watchdog: FSEvents/inotify/ReadDirectoryChanges) - AppFocusObserver (NSWorkspace/xdotool/WinEventHook) - ClipboardObserver (cross-platform polling, hash dedup) - InputTapObserver (Quartz CGEventTap/pynput, 50ms throttle) - ObservationDaemon lifecycle manager (fault isolation) Integration: - Config-driven switch (LEAPFLOW_USE_CUA_DRIVER, default true) - facade.py handshake() adapts to cua-driver capability negotiation - CLI host commands rewritten for cua-driver management - Auto-install diagnostics and graceful degradation Cleanup: - Delete src/leapflow/host/ module (manager, launchd, dev, permissions) - Remove Makefile swift-build/host-* targets, add cua-check - Update README: remove Swift/Xcode deps, add cua-driver prereqs Dependencies: mcp>=1.26.0, watchdog>=3.0 Tests: 126 passed, ruff clean
There was a problem hiding this comment.
Code Review
This pull request migrates the project from the legacy Swift-based macOS host to a cross-platform, pluggable execution backend powered by cua-driver and the Model Control Protocol (MCP). It removes the legacy os_host module and introduces CuaDriverClient along with a suite of passive signal observers (FileSystemObserver, AppFocusObserver, ClipboardObserver, and InputTapObserver) managed by ObservationDaemon. The code review feedback identifies several critical issues: blocking the asyncio event loop with synchronous calls (like time.sleep and blocking I/O) in async contexts, potential task leaks due to uncancelled futures on timeout, highly inefficient polling loops that spawn subprocesses frequently, and incorrect handling of PermissionError when checking process status. Addressing these issues by utilizing executors, caching active window states, and using shutil.which will significantly improve the performance and robustness of the new architecture.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| try: | ||
| fut.result(timeout=5.0) | ||
| except concurrent.futures.TimeoutError: | ||
| logger.warning("cua-driver session shutdown timed out") |
There was a problem hiding this comment.
If the lifecycle task shutdown times out, the task is left running. Cancel fut explicitly to ensure it is terminated.
| try: | |
| fut.result(timeout=5.0) | |
| except concurrent.futures.TimeoutError: | |
| logger.warning("cua-driver session shutdown timed out") | |
| try: | |
| fut.result(timeout=5.0) | |
| except concurrent.futures.TimeoutError: | |
| logger.warning("cua-driver session shutdown timed out") | |
| fut.cancel() |
| try: | ||
| tool_name, tool_args = self._map_to_cua_tool(method, params) | ||
| except _LocalResult as lr: |
There was a problem hiding this comment.
The local handlers in _LOCAL_DISPATCH perform blocking disk I/O or spawn synchronous subprocesses (e.g., clipboard operations, file operations). Calling them directly inside async def call blocks the event loop. Run synchronous handlers in an executor.
| try: | |
| tool_name, tool_args = self._map_to_cua_tool(method, params) | |
| except _LocalResult as lr: | |
| handler = _LOCAL_DISPATCH.get(method) | |
| if handler is not None: | |
| return await asyncio.get_running_loop().run_in_executor( | |
| None, handler, params | |
| ) |
| # Wait up to 5s for process to exit | ||
| for _ in range(50): | ||
| time.sleep(0.1) | ||
| try: | ||
| os.kill(pid, 0) | ||
| except OSError: | ||
| break |
There was a problem hiding this comment.
Calling time.sleep(0.1) inside an async def function blocks the entire event loop. Use await asyncio.sleep(0.1) instead to keep the event loop responsive.
| # Wait up to 5s for process to exit | |
| for _ in range(50): | |
| time.sleep(0.1) | |
| try: | |
| os.kill(pid, 0) | |
| except OSError: | |
| break | |
| # Wait up to 5s for process to exit | |
| import asyncio | |
| for _ in range(50): | |
| await asyncio.sleep(0.1) | |
| try: | |
| os.kill(pid, 0) | |
| except OSError: | |
| break |
| def get_active_app(self) -> Optional[Dict[str, Any]]: | ||
| try: | ||
| # Get active window ID | ||
| wid_result = subprocess.run( | ||
| ["xdotool", "getactivewindow"], | ||
| capture_output=True, text=True, timeout=2, | ||
| ) | ||
| if wid_result.returncode != 0: | ||
| return None | ||
| wid = wid_result.stdout.strip() | ||
|
|
||
| # Get window name | ||
| name_result = subprocess.run( | ||
| ["xdotool", "getactivewindow", "getwindowname"], | ||
| capture_output=True, text=True, timeout=2, | ||
| ) | ||
| window_title = name_result.stdout.strip() if name_result.returncode == 0 else "" | ||
|
|
||
| # Get PID | ||
| pid_result = subprocess.run( | ||
| ["xdotool", "getactivewindow", "getwindowpid"], | ||
| capture_output=True, text=True, timeout=2, | ||
| ) | ||
| pid = int(pid_result.stdout.strip()) if pid_result.returncode == 0 else 0 | ||
|
|
||
| # Get WM_CLASS for app identification | ||
| class_result = subprocess.run( | ||
| ["xprop", "-id", wid, "WM_CLASS"], | ||
| capture_output=True, text=True, timeout=2, | ||
| ) | ||
| app_id = "" | ||
| app_name = "" | ||
| if class_result.returncode == 0: | ||
| # WM_CLASS(STRING) = "instance", "class" | ||
| parts = class_result.stdout.split('"') | ||
| if len(parts) >= 4: | ||
| app_id = parts[3] # class name | ||
| app_name = parts[3] | ||
| elif len(parts) >= 2: | ||
| app_id = parts[1] | ||
| app_name = parts[1] | ||
|
|
||
| return { | ||
| "app_id": app_id, | ||
| "app_name": app_name, | ||
| "pid": pid, | ||
| "window_title": window_title, | ||
| } |
There was a problem hiding this comment.
The polling loop calls get_active_app() every 0.5 seconds. Spawning four subprocesses (xdotool and xprop) every 0.5 seconds is extremely inefficient and will peg the CPU on Linux. Cache the last active window ID (wid) and only fetch the rest of the window details (name, PID, WM_CLASS) when the active window ID actually changes.
def __init__(self) -> None:
self._last_wid: Optional[str] = None
self._cached_info: Optional[Dict[str, Any]] = None
def get_active_app(self) -> Optional[Dict[str, Any]]:
try:
# Get active window ID
wid_result = subprocess.run(
["xdotool", "getactivewindow"],
capture_output=True, text=True, timeout=2,
)
if wid_result.returncode != 0:
return None
wid = wid_result.stdout.strip()
if wid == self._last_wid:
return self._cached_info
# Get window name
name_result = subprocess.run(
["xdotool", "getactivewindow", "getwindowname"],
capture_output=True, text=True, timeout=2,
)
window_title = name_result.stdout.strip() if name_result.returncode == 0 else ""
# Get PID
pid_result = subprocess.run(
["xdotool", "getactivewindow", "getwindowpid"],
capture_output=True, text=True, timeout=2,
)
pid = int(pid_result.stdout.strip()) if pid_result.returncode == 0 else 0
# Get WM_CLASS for app identification
class_result = subprocess.run(
["xprop", "-id", wid, "WM_CLASS"],
capture_output=True, text=True, timeout=2,
)
app_id = ""
app_name = ""
if class_result.returncode == 0:
# WM_CLASS(STRING) = "instance", "class"
parts = class_result.stdout.split('"')
if len(parts) >= 4:
app_id = parts[3] # class name
app_name = parts[3]
elif len(parts) >= 2:
app_id = parts[1]
app_name = parts[1]
info = {
"app_id": app_id,
"app_name": app_name,
"pid": pid,
"window_title": window_title,
}
self._last_wid = wid
self._cached_info = info
return info| "or install: https://github.com/trycua/cua" | ||
| ) | ||
|
|
||
| command, args = _resolve_mcp_invocation(_CUA_DRIVER_CMD) |
There was a problem hiding this comment.
Calling the synchronous, blocking _resolve_mcp_invocation function directly inside an async function blocks the entire event loop. Run it in an executor to keep the loop responsive.
command, args = await asyncio.get_running_loop().run_in_executor(
None, _resolve_mcp_invocation, _get_cua_driver_cmd()
)| try: | ||
| pid = int(pid_file.read_text().strip()) | ||
| # Check if process is alive | ||
| os.kill(pid, 0) | ||
| return pid | ||
| except (ValueError, OSError): | ||
| # Stale or invalid PID file | ||
| try: | ||
| pid_file.unlink() | ||
| except OSError: | ||
| pass | ||
| return None |
There was a problem hiding this comment.
On Unix/macOS, if the daemon process is running under a different user or has restricted privileges, os.kill(pid, 0) can raise a PermissionError. In the current implementation, catching OSError generally will treat this as a stale/invalid PID file and delete it, which is incorrect because the process is actually running. Explicitly catch PermissionError and treat the process as alive to prevent starting duplicate daemons.
| try: | |
| pid = int(pid_file.read_text().strip()) | |
| # Check if process is alive | |
| os.kill(pid, 0) | |
| return pid | |
| except (ValueError, OSError): | |
| # Stale or invalid PID file | |
| try: | |
| pid_file.unlink() | |
| except OSError: | |
| pass | |
| return None | |
| try: | |
| pid = int(pid_file.read_text().strip()) | |
| # Check if process is alive | |
| os.kill(pid, 0) | |
| return pid | |
| except PermissionError: | |
| # Process is alive but we don't have permission to signal it | |
| return pid | |
| except (ValueError, OSError): | |
| # Stale or invalid PID file | |
| try: | |
| pid_file.unlink() | |
| except OSError: | |
| pass | |
| return None |
|
|
||
| def cua_driver_available() -> bool: | ||
| """True if cua-driver binary is discoverable on PATH.""" | ||
| return bool(shutil.which(_CUA_DRIVER_CMD)) |
| "platform": sys.platform, | ||
| "arch": platform.machine(), | ||
| "os_version": platform.version(), | ||
| "cua_driver_cmd": _CUA_DRIVER_CMD, |
|
|
||
| # ── Configuration (all overridable via env) ────────────────────────────────── | ||
|
|
||
| _CUA_DRIVER_CMD = os.environ.get("LEAPFLOW_CUA_DRIVER_CMD", "cua-driver") |
There was a problem hiding this comment.
Evaluating _CUA_DRIVER_CMD at import time prevents it from picking up configuration overrides loaded from .env or settings files later. Define a helper function to read the command dynamically at runtime.
| _CUA_DRIVER_CMD = os.environ.get("LEAPFLOW_CUA_DRIVER_CMD", "cua-driver") | |
| def _get_cua_driver_cmd() -> str: | |
| return os.environ.get("LEAPFLOW_CUA_DRIVER_CMD", "cua-driver") |
| def __init__(self) -> None: | ||
| self._cmd: Optional[list[str]] = None | ||
| # Detect available tool | ||
| for cmd in [ | ||
| ["xclip", "-selection", "clipboard", "-o"], | ||
| ["xsel", "--clipboard", "--output"], | ||
| ]: | ||
| try: | ||
| subprocess.run( | ||
| cmd, capture_output=True, timeout=1, | ||
| ) | ||
| self._cmd = cmd | ||
| break | ||
| except FileNotFoundError: | ||
| continue |
There was a problem hiding this comment.
Spawning a subprocess (subprocess.run) during object initialization just to check if a command exists is inefficient and can cause side effects or hang. Use shutil.which to check if the binaries are available on the system PATH instead.
| def __init__(self) -> None: | |
| self._cmd: Optional[list[str]] = None | |
| # Detect available tool | |
| for cmd in [ | |
| ["xclip", "-selection", "clipboard", "-o"], | |
| ["xsel", "--clipboard", "--output"], | |
| ]: | |
| try: | |
| subprocess.run( | |
| cmd, capture_output=True, timeout=1, | |
| ) | |
| self._cmd = cmd | |
| break | |
| except FileNotFoundError: | |
| continue | |
| def __init__(self) -> None: | |
| self._cmd: Optional[list[str]] = None | |
| import shutil | |
| if shutil.which("xclip"): | |
| self._cmd = ["xclip", "-selection", "clipboard", "-o"] | |
| elif shutil.which("xsel"): | |
| self._cmd = ["xsel", "--clipboard", "--output"] |
No description provided.