feat(cua-driver-rs): daemon session identity — own + clean up session-scoped recording & config - #1776
Merged
Merged
Conversation
…roxy-exit + idle-TTL) + default record_video=false (#1764) The serve daemon's recording is a process-global singleton with no connection ownership. When an MCP client started a record_video recording and disconnected without calling stop_recording, the ScreenCaptureKit SCStream kept capturing the full display at native res / 30fps / H.264 indefinitely — ~6 GB/h to disk until the daemon was killed. Why the first attempt (per-connection teardown in serve.rs) was wrong: the shipped daemon-proxy transport forwards EVERY tools/call via send_request, which opens a brand-new short-lived UnixStream per call and closes it the instant the call returns. So the daemon per-connection task handles exactly ONE request then hits EOF. Putting teardown there fired milliseconds after start_recording (before any turn) and BROKE recording on the primary path. 'connection close' != 'session end'. That serve.rs hunk is fully reverted here. The correct seam is the MCP proxy process, whose lifetime == the MCP session: 1. Proxy-exit hook (proxy.rs, primary fix). run_proxy tracks whether THIS session has an outstanding recording (flag flipped only on a successful start/stop_recording forward; plain bool is safe because the read loop is strictly sequential). When stdin hits EOF (the real client-disconnect seam) the proxy sends one best-effort stop_recording DaemonRequest to the daemon socket, then exits. The daemon services it like any other per-call connection. 2. Idle-TTL backstop (serve.rs, defense-in-depth). A daemon-global task auto-stops recording after RECORDING_IDLE_TTL_SECS (default 300s, overridable via CUA_DRIVER_RS_RECORDING_IDLE_TTL_SECS) of zero tool-call activity. Covers the case the proxy can't run its hook (SIGKILL/crash, or a non-proxy client that dies holding a connection). Keyed on call activity, not connection liveness, so an actively-used session is never reaped. stop() is idempotent, so racing the proxy-exit hook is benign. Added to both unix and Windows run_serve for parity. 3. record_video now defaults to false (recording_tools.rs, kept from the prior attempt). A full-display native-res 30fps recording defaulting on for every start_recording was a sharp footgun. The legacy CLI recording start path keeps video on (asymmetry intentional). Tool/schema text, recording.rs doc-comment, and the /docs mcp-tools reference updated. Verified against a freshly built daemon on an isolated socket (/tmp/wf1764b), all 4 checks pass: (a) recording survives multiple separate per-call daemon connections (no premature teardown); (b) closing the real cua-driver mcp proxy's stdin stops the recording within ~3s; (c) idle-TTL auto-stops after the TTL with no activity; (d) start_recording with no record_video arg produces no recording.mp4. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…eo comments (#1764 review nits) Addresses the three minor/nit review findings on PR #1775 (comment-only, no behavior change): - proxy.rs: the proxy-exit teardown block is reached on a clean stdin EOF (the normal MCP-client disconnect); reword the comment to stop implying it also covers the I/O-error `?` path, and note that path is backstopped by the daemon-side recording idle-TTL. - recording.rs: fix the stale `configure()` doc-comment that said it defaults record_video "to match the new surface" — the MCP tool now defaults video OFF; the legacy CLI path forces it on. - serve.rs: note the idle backstop's 30s tick granularity (sub-30s TTL overrides still fire no sooner than ~30s). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…stop so it cannot stop another session's recording (#1764) The daemon recorder is a global singleton. With the proxy-exit auto-stop added in #1775, this sequence could stop a recording its owner no longer held: client A start_recording (owns it), client B start_recording (clobbers A, now owns the recorder), then A disconnects and its proxy-exit hook sent an unconditional stop_recording -> B's recording stopped. Add a monotonically-increasing `generation` token to RecordingSession, bumped on every successful start(). start_recording surfaces it in structuredContent; the proxy captures the generation it started with and passes it back on the proxy-exit stop_recording. RecordingSession::stop() gains an optional expected-generation guard (race-free inside the lock): a stale token is a silent no-op, leaving the newer owner's recording running. Manual stop_recording (no generation) and the idle-TTL backstop stay unconditional. Verified live against a real daemon+proxy on an isolated socket: A(gen1)/B(gen2)/A-exit leaves B's gen2 recording enabled; B-exit stops it; single-session exit stops; manual stop with no generation stops; default record_video stays off; idle-TTL still reaps after inactivity. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-scoped recording & config by a proxy-minted session_id The cua-driver daemon (serve.rs) is one shared process: every `cua-driver mcp` proxy connects to it and shares its ToolState. Any "set once, affects later calls" daemon state is therefore multi-tenant and clobbered across concurrent sessions — the trajectory recorder is a singleton, and set_config mutates a process-global + disk-persisted DriverConfig. Two MCP sessions racing set_config clobber each other AND flip the on-disk default under everyone; one session's disconnect could stop another's recording. This adds a proxy-minted session identity carried in the daemon request envelope, used to OWN and CLEAN UP session-scoped state: - Envelope: optional `session_id` on DaemonRequest (skip_serializing_if = Option::is_none, so absent == byte-identical legacy wire; serde defaults a missing field to None). New `session_end` lifecycle method reuses the same envelope. The daemon injects `_session_id` into tool args before invoke (both Unix + Windows branches); tools read it via the existing ArgsExt, exactly like cursor_id. No Tool trait change. - Recording ownership: RecordingSession gains an `owner: Option<String>`; start() stamps it from `_session_id`; stop_owner(requester) — None = unconditional (manual/CLI/idle-TTL), Some==owner = stop, Some!=owner = no-op. Supersedes the #1775 generation token wholesale (a stable owner identity that doubles as the config key, not a monotonic counter). - Config-per-session (macOS): SessionConfigRegistry holds in-memory overrides keyed by session_id, layered over the global. A named MCP session's set_config writes only its override (no global write, no disk); the anonymous CLI / one-shot path keeps writing + persisting the global default. get_config / get_window_state / click resolve effective = call-arg > session > global. - Cleanup: proxy mints the id once (dep-free pid+nanos, no uuid crate) and sends one best-effort `session_end` on stdin EOF; the daemon drops that session's recording (stop_owner) + config overrides (platform session-end hooks). Old daemons return "Unknown method" and the proxy swallows it (graceful degrade). - `_session_id` and any `_`-prefixed key are stripped before a turn is recorded, so the UUID never lands in action.json. Backward-compatible: absent session_id == anonymous/global session (today's behavior). One-shot `cua-driver call` is its own ephemeral anonymous session. The daemon drives ONE physical machine, so this solves state ownership + cleanup, not safe concurrent screen/input control. Deferred to follow-ups (design-noted): cursor session-scoping, PiP (process- global by design), a generic per-session idle reaper for SIGKILLed proxies, Windows/Linux set_config parity, and the clean invoke_with_session trait API. Verified live on an isolated socket with two real `cua-driver mcp` proxies: config isolation (A=vision / B=som, persisted default unchanged), recording ownership (A+B start, A disconnect → B survives; B disconnect → stops; A-only disconnect → stops; manual stop unconditional), backward-compat one-shot call, and `_session_id` stripped from recorded turns. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThis PR implements session-scoped recording ownership and per-session configuration isolation for daemon-proxy mode. It adds session lifecycle hooks, owns recordings with optional session identifiers, extends the daemon protocol with session_id forwarding, implements session_end lifecycle handlers, and introduces in-memory per-session config overrides layered over persisted global config. ChangesSession-scoped daemon state and lifecycle
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
f-trycua
marked this pull request as ready for review
May 31, 2026 02:03
This was referenced May 31, 2026
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The multi-tenant problem
The cua-driver daemon (
serve.rs) is one shared process: everycua-driver mcpproxy connects to it and shares itsToolState. Any "set once, affects later calls" daemon state is therefore multi-tenant and gets clobbered across concurrent MCP sessions:ToolRegistry.recordingis a singleton. Session A's disconnect could stop session B's recording.set_configdoesconfig.write()AND persists to~/.cua-driver/config.json. A setsvision, B setssom→ clobber, and the on-disk default flips under everyone.The daemon drives one physical machine, so this is a state ownership + cleanup problem, not safe concurrent screen/input control (two sessions clicking at once still contend for one desktop — out of scope, kept honest).
The session_id model
A proxy-minted session identity (one per
cua-driver mcpprocess) carried in the daemon request envelope, used to OWN and CLEAN UP session-scoped state.DaemonRequestgains an optionalsession_id(skip_serializing_if = Option::is_none, so the absent case is byte-identical on the wire; serde defaults a missing field toNone). A newsession_endlifecycle method reuses the same envelope. The daemon injects_session_idinto the tool args right beforeinvoke(Unix + Windows mirrors); tools read it via the existingArgsExt, exactly likecursor_id— noTooltrait change, no schema whitelist (the daemon never validates args againstinput_schema).RecordingSessiongainsowner: Option<String>;start()stamps it from_session_id;stop_owner(requester)isNone= unconditional (manual/CLI/idle-TTL),Some==owner= stop,Some!=owner= no-op. Supersedes the fix(cua-driver-rs)(macos): tear down recording on client disconnect + default record_video=false (#1764) #1775 generation token wholesale — a stable owner identity that doubles as the config key, instead of a monotonic counter.SessionConfigRegistryholds in-memory overrides keyed bysession_id, layered over the global. A named MCP session'sset_configwrites only its override (no global write, no disk); the anonymous CLI / one-shot path keeps writing + persisting the global default.get_config/get_window_state/clickresolve effective = call-arg > session > global.pid+nanos, nouuidcrate) and sends one best-effortsession_endon stdin EOF; the daemon drops that session's recording (stop_owner) + config overrides (platform session-end hooks)._session_idand any_-prefixed key are stripped before a turn is recorded, so the UUID never lands inaction.json.In scope vs deferred
In: the envelope +
session_end, daemon-side_session_idinjection (both platforms), proxy minting/teardown, recording ownership (replacing the #1775 token), and config-per-session for macOS.Deferred (design-noted TODOs):
CursorRegistryneeds aremove/overlay-disable that doesn't exist yet.session_endonly fires on graceful EOF). The daemon-global recording idle-TTL is an acceptable interim backstop; there is no merged config TTL yet, so a SIGKILLed session's config overrides linger until daemon restart — a small bounded in-memory leak.set_configparity — they have their ownDriverConfig; this PR scopes the config split to macOS.invoke_with_sessiontrait/context API — YAGNI; inject-via-args avoids touching every Tool impl across 3 platform crates.Verification (live, isolated socket, two real
cua-driver mcpproxies)set_config capture_mode=vision, Bset_config capture_mode=som; A'sget_config→ vision, B's → som, concurrently, no clobber.~/.cua-driver/config.jsoncapture_mode stayed ax (neither named-session write touched disk). ✅start_recording, Bstart_recording(clobbers the singleton); close A → recording still enabled,owner= B's id; close B → stops. A-only start + disconnect → stops. ✅recording stop(session_id=None→stop_owner(None)). ✅cua-driver call get_config(no session) returns the global;call set_config capture_mode=visionwrites the persisted global default (survives a daemon reboot viaload_driver_config()). ✅move_cursorturn'saction.jsoncarries only{x, y};_session_idwas injected, used, and stripped. ✅cargo build -p cua-driverclean;cargo test -p cua-driver -p cua-driver-coreintroduces no new failures beyond the 5 pre-existingmcp_protocol_testones (verified identical set on the base branch).Notes
session_endmethod, new publiccore::sessionhook surface, andget_config-from-a-named-session now reports effective-per-session values rather than the raw global.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Documentation