Multi-agent exclusive desktop leases + idle VNC reaper - #10
Conversation
Agents get exclusive seats via ensure_desktop(owner_id): reuse own lease, claim idle seats, or auto-spawn a new Xvnc/noVNC pair on a free port. Leases expire and a background reaper frees the lock and stops pool VNC on idle or process exit (long-lived MCP hosts). Default geometry is 2560x1440 for side-by-side work. Windows gets a limited hybrid backend so the server imports cleanly; CI runs lint/unit on Linux/macOS/Windows plus a Linux dual-owner e2e smoke.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_b35fb5e2-a704-46b0-a8b5-2b4ff2a55378) |
|
Warning Review limit reached
Next review available in: 32 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
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 |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| return _pid_alive(ws) | ||
| return False | ||
|
|
||
| async def acquire_desktop(self, geometry) -> dict: | ||
| async def ensure_desktop(self, geometry: str = DEFAULT_GEOMETRY, owner_id: str = "") -> dict: | ||
| # Single real desktop — lease is advisory; second agent is warned but can share. |
There was a problem hiding this comment.
Bug: In X11Backend.list_desktops, the result of a session reap is discarded and the session pool is re-read outside the lock, creating a race condition.
Severity: MEDIUM
Suggested Fix
Do not discard the mutated pool dictionary after _reap_pool_locked is called. Use this in-memory pool to build the session list, which avoids the race condition and the unnecessary file read.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: mcp-server/server.py#L1396-L1400
Potential issue: In `X11Backend.list_desktops`, an opportunistic reap of dead sessions
is performed inside a lock to ensure the returned list is 'honest'. However, the result
of this operation is discarded. The code then releases the lock and re-reads the session
pool from disk. This creates a time-of-check-to-time-of-use (TOCTOU) race condition
where concurrent processes can add new sessions between the lock release and the
re-read, defeating the purpose of the reap and making the returned list inconsistent
with the state immediately after the cleaning operation.
Did we get this right? 👍 / 👎 to inform future reviews.
| os.kill(pid, signal.SIGKILL) | ||
| except OSError: | ||
| pass | ||
| _clear_lease(sess) | ||
| pool[session_id] = sess | ||
| _write_pool(pool) | ||
| self._bridge_pid = None | ||
| _LAST_SHOT.pop(sess["display"], None) | ||
| pool.pop(session_id, None) |
There was a problem hiding this comment.
Bug: MacBackend.release_desktop incorrectly deletes the session record due to a double-write and holds a file lock across an await, causing lock contention.
Severity: MEDIUM
Suggested Fix
Remove the second pool.pop(session_id, None) and _write_pool(pool) call to prevent the session from being deleted. To fix the lock contention, move the entire lock-protected synchronous code block into a separate function and run it in a thread pool executor using run_in_executor, similar to the pattern used in X11Backend.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: mcp-server/server.py#L1511-L1519
Potential issue: The `MacBackend.release_desktop` method contains two distinct issues.
First, it performs a double write: it clears a session's lease and writes the pool to
disk, but then immediately removes the session from the pool and writes to disk again.
This incorrectly deletes the session record, causing `list_desktops` to report zero
sessions. Second, it holds a file lock across an `await asyncio.sleep(0.3)` call. This
blocks any other concurrent operations that require the same lock for the duration of
the sleep, leading to performance degradation and potential timeouts under load.
Also affects:
mcp-server/server.py:1458
Did we get this right? 👍 / 👎 to inform future reviews.
fcntl is Unix-only; CI on windows-latest failed at import. Use msvcrt byte locks on Windows and fcntl.flock elsewhere, put pool files under the OS temp dir on Windows, and only install python-xlib on Linux. WinBackend ensure no longer overwrites another owner's unexpired lease.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_2a29a359-9001-4a3e-9bc5-c79d1a1b51a6) |
argparse -h failed on windows-latest under cp1252 because help text used U+2192. Keep the run help string plain ASCII.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_152e5513-e0bd-48f1-8b2b-71c5fe812ec7) |
1 participant
Summary
Multi-agent safe desktops for concurrent agents on Linux (pool), with portable leases on macOS/Windows.
ensure_desktop(owner_id)reuses the agent lease, claims idle seats, or auto-spawns a new Xvnc/noVNC on the next free port when others are busyowner_id+lease_untilunder the existing file-locked poolRELEASE_ON_EXIT=0so one-shot ensure keeps seats)ensure,heartbeat,reap,acquire --force-newTest plan
ruff check/format,pytest tests/, dual-owner ensure →:50+:51, reuse owner, release kills VNCNote
Medium Risk
Large changes to pool lifecycle (spawn, reap, process kill, cross-process locks) affect concurrent agents and host resource usage; Windows/macOS paths are mostly advisory leases with different isolation than Linux.
Overview
Adds multi-agent exclusive desktop leasing on the existing file-locked Xvnc pool, bumps the plugin to 0.2.0, and wires CI (ruff, pytest, Linux dual-owner ensure smoke).
MCP server: New tools
ensure_desktop,heartbeat_desktop, andreap_idle_desktops. Pool sessions now carryowner_id+lease_until;ensure_desktopreuses the caller’s live lease, claims idle/expired seats, or auto-spawns the next free display when others are busy (never steals a live lease).acquire_desktopdefaults to ensure behavior;force_newkeeps always-spawn. A background reaper and exit/SIGTERM handling free leases and kill pool VNC (Xvnc + websockify); host default display is not reaped. Default geometry is 2560x1440.WinBackendprovides lease-only portability; pool lock uses msvcrt on Windows. CLI setsDESKTOP_ACT_RELEASE_ON_EXIT=0so one-shot invocations don’t tear down desktops immediately.CLI: Subcommands
ensure,heartbeat,reap;runuses ensure instead of acquire;--owner-id/--geometry/--force-new.Docs & packaging: README, skill, and CLI README updated for the multi-agent pattern.
pyproject.tomladded;python-xlibgated to Linux only.Reviewed by Cursor Bugbot for commit 6d373b8. Bugbot is set up for automated code reviews on this repo. Configure here.