chore(cua): sync driver v0.2.0#1671
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR syncs the vendored CUA driver from v0.1.5 to v0.2.0, introducing focus-suppression leak resilience via deadline-based reaping, MCP daemon proxying, window side-effect detection, improved app resolution, streaming capture fallback for macOS 26.4.x, and new diagnostic commands. ChangesCUA Driver v0.2.0 Upstream Sync
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (1)
plugins/cua/vendor/cua-driver/source/Tests/FocusStealPreventerTests/FocusStealPreventerTests.swift (1)
115-229: ⚡ Quick winAdd a regression test for janitor shutdown/restart.
These tests prove
_forceReapForTesting()and explicit release paths, but they don't cover the newjanitorTasklifecycle. A race where the janitor stops onactiveCount == 0and a subsequent lease arrives beforeclearJanitor()runs would still pass this suite and reintroduce idle leak risk.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/cua/vendor/cua-driver/source/Tests/FocusStealPreventerTests/FocusStealPreventerTests.swift` around lines 115 - 229, Add a regression test that reproduces the janitor shutdown/restart race: create a SystemFocusStealPreventer with a short janitorIntervalNs, add a suppression (via beginSuppression or leaseSuppression) and then release it so activeCount reaches 0 and the janitor stops, then immediately create a new lease (leaseSuppression) before clearJanitor() would complete; assert the new entry is not lost (activeCount == 1) and that the janitor lifecycle is correctly resumed/handled (e.g. janitorTask is running or future reaps succeed). Use the existing helpers (_forceReapForTesting, beginSuppression, leaseSuppression, endSuppression, activeCount) and name the test to indicate janitor shutdown/restart regression.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@plugins/cua/vendor/cua-driver/source/scripts/build/build-release-notarized.sh`:
- Around line 63-72: The script currently only checks CUA_DRIVER_PREBUILT_BINARY
for non-empty and silences swift build output; update the block that sets
BUILT_BINARY so it first validates that CUA_DRIVER_PREBUILT_BINARY is an
absolute path and points to an existing, readable (and executable if needed)
file and if not log an "error" and exit non-zero before any cp/sign steps; for
the build path, run swift build without discarding output (or capture
stdout/stderr) and immediately check its exit status ($?) — on failure log the
captured output with process context via log "error" and exit non-zero; ensure
BUILT_BINARY is only set after successful validation or successful build so
subsequent steps never operate on an invalid path.
In `@plugins/cua/vendor/cua-driver/source/scripts/install.sh`:
- Around line 154-168: The installer’s baked fallback CUA_DRIVER_BAKED_VERSION
("0.1.9") is stale and will override newer releases; update the install.sh logic
by setting CUA_DRIVER_BAKED_VERSION to the vendored driver's actual version
(match the value in Sources/CuaDriverCore/CuaDriverCore.swift public static let
version) or remove the baked fallback so TAG selection falls back to GitHub
Releases; ensure the variable CUA_DRIVER_BAKED_VERSION and the TAG assignment
logic (the elif branch that sets TAG="${TAG_PREFIX}${CUA_DRIVER_BAKED_VERSION}")
are updated accordingly.
In
`@plugins/cua/vendor/cua-driver/source/Sources/CuaDriverCLI/CuaDriverCommand.swift`:
- Around line 507-515: The daemon relaunch ignores a custom --socket because
launchDaemonViaOpen() is invoked without the socket override, causing the new
daemon to bind the default socket while waitForDaemon(socketPath:timeout:) polls
the custom path; fix by adding a socketPath parameter to launchDaemonViaOpen (or
an overload) and include the socket argument when composing the open/--args
invocation so the spawned daemon is started with the same socketPath, then
update the call sites (the invocation in the block using socketPath and the
other call around lines 545-555) to pass the socketPath through; ensure
waitForDaemon(socketPath:timeout:) still polls the same path.
In
`@plugins/cua/vendor/cua-driver/source/Sources/CuaDriverCLI/DoctorCommand.swift`:
- Around line 101-107: probeAXTree(pid:) currently returns true if
AXUIElementCopyAttributeValue for kAXChildrenAttribute succeeds even when the
children list is empty; change the logic to require both err == .success and
that the returned value contains at least one child. After calling
AXUIElementCopyAttributeValue(app, kAXChildrenAttribute as CFString, &value)
cast/unwrap value to an array type (e.g., CFArray/NSArray or Swift [Any]) and
ensure its count > 0 before returning true; otherwise return false. Keep checks
around AXUIElementCopyAttributeValue, handle optional unwrapping safely, and
refer to probeAXTree, AXUIElementCopyAttributeValue, and kAXChildrenAttribute
while making the change.
In
`@plugins/cua/vendor/cua-driver/source/Sources/CuaDriverCore/Apps/AppLauncher.swift`:
- Around line 265-309: The installed-bundle scan in the AppLauncher pass-3 code
uses bundle.infoDictionary (non-localized) to read
CFBundleDisplayName/CFBundleName, so cold-launch matching misses locale-specific
names; update the displayName lookup in the loop that builds displayName (the
code that references bundle.infoDictionary?["CFBundleDisplayName"] and
["CFBundleName"]) to first check bundle.localizedInfoDictionary for those keys
and only fall back to bundle.infoDictionary if localized entries are missing,
preserving the existing final fallback to the path stem and the subsequent stem
comparison.
In
`@plugins/cua/vendor/cua-driver/source/Sources/CuaDriverCore/Capture/WindowCapture.swift`:
- Around line 273-275: The backoff sleep in WindowCapture (inside the retry loop
in WindowCapture.swift) currently uses `try? await Task.sleep(...)` which
swallows Task cancellation; change it to use `try await Task.sleep(...)` so
CancellationError is propagated (or catch and rethrow CancellationError
explicitly) to ensure cancelled tasks don't proceed to the subsequent capture
attempt; locate the sleep call near the guarded `guard case .streamingFailed =
classified` and update it accordingly.
In
`@plugins/cua/vendor/cua-driver/source/Sources/CuaDriverCore/Focus/SystemFocusStealPreventer.swift`:
- Around line 345-359: When the janitor loop decides to stop, don’t simply break
and leave janitorTask non-nil while the detached task hops back into the actor;
instead, await entry into the actor and clear the janitor immediately so a
concurrent startJanitorIfNeeded() can see janitorTask become nil and restart.
Concretely, inside the Task.detached block in startJanitorIfNeeded(), replace
the "if await self?.shouldStopJanitor() ?? true { break }" behavior with an
awaited call to self?.clearJanitor() (or otherwise set janitorTask = nil on the
actor) and then return, and remove the duplicated clear at the end so
clearJanitor() is performed atomically when stopping; this ensures
startJanitorIfNeeded(), janitorTask, shouldStopJanitor(), and clearJanitor()
coordinate to avoid the startup gap.
In
`@plugins/cua/vendor/cua-driver/source/Sources/CuaDriverServer/Tools/ClickTool.swift`:
- Around line 255-256: The side-effect snapshot is taken too early (before
fallible pre-checks) causing the wildcard suppressor to be armed even when
lookup/debug-image/coordinate-resolution fail; move the call to
WindowChangeDetector.snapshot() so it runs after all pre-checks that can fail
(e.g., after the lookup/debug-image/coordinate-resolution/coordinate-resolution
logic in ClickTool) and immediately before the AX/CGEvent dispatch; ensure the
same change is applied to the duplicate snapshot location around lines 460-463
and verify SuppressionLease.deinit's detached cleanup remains the fallback on
earlier failures.
In
`@plugins/cua/vendor/cua-driver/source/Sources/CuaDriverServer/Tools/LaunchAppTool.swift`:
- Around line 56-64: resolveLaunchURL(_:) currently only treats "http", "https",
and "file" as URL schemes so "about:blank" falls into the file-path branch;
update resolveLaunchURL(_:) (in LaunchAppTool.swift) to treat the "about" scheme
as a valid URL scheme (or explicitly special-case "about:blank") and return
URL(string: value) for it instead of trying to resolve a file path, ensuring
callers using urls=["about:blank"] produce a proper about: URL that opens a
blank browser tab/window.
In
`@plugins/cua/vendor/cua-driver/source/Sources/CuaDriverServer/Tools/ListWindowsTool.swift`:
- Around line 113-126: The current early return in ListWindowsTool (inside the
pidFilter && windows.isEmpty branch) builds a plain text warning and returns a
CallTool.Result with only text content, breaking the tool's expected response
shape; change that return to include the same warning text but also provide the
usual structuredContent fields (e.g., an empty or default structuredContent
containing "windows" as an empty array and "current_space_id" as nil/string as
the tool normally returns) so CallTool.Result preserves the expected schema;
update the return construction in that branch (where the warning is created and
returned) to include both .text content and a properly formed structuredContent
payload.
In
`@plugins/cua/vendor/cua-driver/source/Sources/CuaDriverServer/Tools/WindowChangeDetector.swift`:
- Around line 180-181: In WindowChangeDetector's detectChanges polling loop (the
pollLoop that uses Date() < deadline and sleep with pollInterval), don't swallow
CancellationError with try?; instead perform try await Task.sleep(...) and catch
CancellationError specifically to break out of the pollLoop so execution
proceeds to the existing await lease.release(); leave other errors to be handled
as before (rethrow or log) so the lease is released immediately on task
cancellation.
In
`@plugins/cua/vendor/cua-driver/source/Tests/integration/test_click_opens_new_window.py`:
- Around line 85-91: The startup wait loop blocks on proc.stdout.readline(),
which can hang if FocusMonitorApp never emits a newline; replace the blocking
call with non-blocking polling: use select.select (or equivalent) on proc.stdout
(proc.stdout.fileno()) inside the loop with a short timeout (e.g., 0.1s) and
only call proc.stdout.readline() when the stream is ready, keep the existing
40-iteration overall timeout and the FOCUS_PID detection
(line.startswith("FOCUS_PID=") and int(line.split("=",1)[1])), and retain
proc.terminate() + RuntimeError when the timeout expires.
In
`@plugins/cua/vendor/cua-driver/source/Tests/integration/test_hidden_app_capture.py`:
- Around line 121-122: The test uses a hardcoded fake_pid (fake_pid = 99999)
which can collide on high-pid systems; change both occurrences where fake_pid is
set before calling self.client.call_tool("list_windows", {"pid": fake_pid}) so
fake_pid is computed deterministically from current process ids (e.g., fake_pid
= max(current_pids) + a large offset) instead of a constant; update the setup in
test_hidden_app_capture.py to compute current_pids (via the system PID list) and
derive fake_pid = max(...) + offset and reuse that derived fake_pid for both
calls to list_windows to remove flakiness.
---
Nitpick comments:
In
`@plugins/cua/vendor/cua-driver/source/Tests/FocusStealPreventerTests/FocusStealPreventerTests.swift`:
- Around line 115-229: Add a regression test that reproduces the janitor
shutdown/restart race: create a SystemFocusStealPreventer with a short
janitorIntervalNs, add a suppression (via beginSuppression or leaseSuppression)
and then release it so activeCount reaches 0 and the janitor stops, then
immediately create a new lease (leaseSuppression) before clearJanitor() would
complete; assert the new entry is not lost (activeCount == 1) and that the
janitor lifecycle is correctly resumed/handled (e.g. janitorTask is running or
future reaps succeed). Use the existing helpers (_forceReapForTesting,
beginSuppression, leaseSuppression, endSuppression, activeCount) and name the
test to indicate janitor shutdown/restart regression.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d623db3f-3d0b-4d87-b2d5-3f287e7a591d
📒 Files selected for processing (31)
docs/issues/cua-driver-v0-2-0-sync/plan.mddocs/issues/cua-driver-v0-2-0-sync/spec.mddocs/issues/cua-driver-v0-2-0-sync/tasks.mdplugins/cua/vendor/cua-driver/source/.bumpversion.cfgplugins/cua/vendor/cua-driver/source/Package.swiftplugins/cua/vendor/cua-driver/source/Sources/CuaDriverCLI/BundleHelpers.swiftplugins/cua/vendor/cua-driver/source/Sources/CuaDriverCLI/CuaDriverCommand.swiftplugins/cua/vendor/cua-driver/source/Sources/CuaDriverCLI/Docs/CLIDocExtractor.swiftplugins/cua/vendor/cua-driver/source/Sources/CuaDriverCLI/DoctorCommand.swiftplugins/cua/vendor/cua-driver/source/Sources/CuaDriverCLI/ServeCommand.swiftplugins/cua/vendor/cua-driver/source/Sources/CuaDriverCore/Apps/AppLauncher.swiftplugins/cua/vendor/cua-driver/source/Sources/CuaDriverCore/Capture/WindowCapture.swiftplugins/cua/vendor/cua-driver/source/Sources/CuaDriverCore/CuaDriverCore.swiftplugins/cua/vendor/cua-driver/source/Sources/CuaDriverCore/Focus/FocusGuard.swiftplugins/cua/vendor/cua-driver/source/Sources/CuaDriverCore/Focus/SystemFocusStealPreventer.swiftplugins/cua/vendor/cua-driver/source/Sources/CuaDriverCore/Windows/WindowEnumerator.swiftplugins/cua/vendor/cua-driver/source/Sources/CuaDriverServer/CuaDriverMCPServer.swiftplugins/cua/vendor/cua-driver/source/Sources/CuaDriverServer/ToolRegistry.swiftplugins/cua/vendor/cua-driver/source/Sources/CuaDriverServer/Tools/ClickTool.swiftplugins/cua/vendor/cua-driver/source/Sources/CuaDriverServer/Tools/GetWindowStateTool.swiftplugins/cua/vendor/cua-driver/source/Sources/CuaDriverServer/Tools/LaunchAppTool.swiftplugins/cua/vendor/cua-driver/source/Sources/CuaDriverServer/Tools/ListWindowsTool.swiftplugins/cua/vendor/cua-driver/source/Sources/CuaDriverServer/Tools/ScreenshotTool.swiftplugins/cua/vendor/cua-driver/source/Sources/CuaDriverServer/Tools/WindowChangeDetector.swiftplugins/cua/vendor/cua-driver/source/Tests/FocusStealPreventerTests/FocusStealPreventerTests.swiftplugins/cua/vendor/cua-driver/source/Tests/integration/test_app_name_locale_fallback.pyplugins/cua/vendor/cua-driver/source/Tests/integration/test_click_opens_new_window.pyplugins/cua/vendor/cua-driver/source/Tests/integration/test_hidden_app_capture.pyplugins/cua/vendor/cua-driver/source/scripts/build/build-release-notarized.shplugins/cua/vendor/cua-driver/source/scripts/install.shplugins/cua/vendor/cua-driver/upstream.json
Comment on lines
+63
to
+72
| # Build or use a prebuilt binary (e.g. a pre-lipo'd universal binary from CI). | ||
| # Set CUA_DRIVER_PREBUILT_BINARY to an absolute path to skip swift build entirely. | ||
| if [ -n "${CUA_DRIVER_PREBUILT_BINARY:-}" ]; then | ||
| log "essential" "Using prebuilt binary: $CUA_DRIVER_PREBUILT_BINARY" | ||
| BUILT_BINARY="$CUA_DRIVER_PREBUILT_BINARY" | ||
| else | ||
| log "essential" "Building release version..." | ||
| swift build -c release --product cua-driver > /dev/null | ||
| BUILT_BINARY=".build/release/cua-driver" | ||
| fi |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="plugins/cua/vendor/cua-driver/source/scripts/build/build-release-notarized.sh"
echo "== $FILE =="
nl -ba "$FILE" | sed -n '1,140p'Repository: ThinkInAIXYZ/deepchat
Length of output: 210
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="plugins/cua/vendor/cua-driver/source/scripts/build/build-release-notarized.sh"
echo "== $FILE (lines 55-100) =="
# print with line numbers using awk
awk 'NR>=55 && NR<=100 {printf "%d:%s\n", NR, $0}' "$FILE"
echo
echo "== Search: CUA_DRIVER_PREBUILT_BINARY =="
rg -n "CUA_DRIVER_PREBUILT_BINARY" "$FILE" || true
echo
echo "== Search: BUILT_BINARY usage =="
rg -n "BUILT_BINARY" "$FILE" || true
echo
echo "== File tail (last 80 lines) =="
awk 'NR>=( $(wc -l < "$FILE") - 80) {printf "%d:%s\n", NR, $0}' "$FILE"Repository: ThinkInAIXYZ/deepchat
Length of output: 13045
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="plugins/cua/vendor/cua-driver/source/scripts/build/build-release-notarized.sh"
echo "== Top of file (lines 1-60) =="
awk 'NR<=60 {printf "%d:%s\n", NR, $0}' "$FILE"
echo
echo "== Check: set -e / pipefail presence =="
rg -n "set -e|set -o pipefail|pipefail|set -u" "$FILE" || true
echo
echo "== Around cp -f "$BUILT_BINARY" (lines 74-90) =="
awk 'NR>=72 && NR<=90 {printf "%d:%s\n", NR, $0}' "$FILE"Repository: ThinkInAIXYZ/deepchat
Length of output: 1669
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="plugins/cua/vendor/cua-driver/source/scripts/build/build-release-notarized.sh"
echo "== set -e / pipefail occurrences =="
rg -n "set -e|pipefail|set -o" "$FILE" || true
echo
echo "== Around lines 60-90 (prebuilt, swift build, cp) =="
awk 'NR>=60 && NR<=95 {printf "%d:%s\n", NR, $0}' "$FILE"Repository: ThinkInAIXYZ/deepchat
Length of output: 1846
Validate CUA_DRIVER_PREBUILT_BINARY and fail fast on swift build failures.
CUA_DRIVER_PREBUILT_BINARYis only checked for non-empty (lines 65-67) even though the comment requires an absolute path; missing/non-executable paths flow intocp(line 83) and can fail later during signing/notarization.swift build(line 70) failure isn’t handled inline, and its output is discarded (> /dev/null), so errors can surface later with less actionable logs.
Proposed patch
# Build or use a prebuilt binary (e.g. a pre-lipo'd universal binary from CI).
# Set CUA_DRIVER_PREBUILT_BINARY to an absolute path to skip swift build entirely.
if [ -n "${CUA_DRIVER_PREBUILT_BINARY:-}" ]; then
+ if [[ "$CUA_DRIVER_PREBUILT_BINARY" != /* ]]; then
+ log "error" "CUA_DRIVER_PREBUILT_BINARY must be an absolute path"
+ exit 1
+ fi
+ if [ ! -x "$CUA_DRIVER_PREBUILT_BINARY" ]; then
+ log "error" "Prebuilt binary missing or not executable: $CUA_DRIVER_PREBUILT_BINARY"
+ exit 1
+ fi
log "essential" "Using prebuilt binary: $CUA_DRIVER_PREBUILT_BINARY"
BUILT_BINARY="$CUA_DRIVER_PREBUILT_BINARY"
else
log "essential" "Building release version..."
- swift build -c release --product cua-driver > /dev/null
+ if ! swift build -c release --product cua-driver > /dev/null; then
+ log "error" "swift build failed"
+ exit 1
+ fi
BUILT_BINARY=".build/release/cua-driver"
fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@plugins/cua/vendor/cua-driver/source/scripts/build/build-release-notarized.sh`
around lines 63 - 72, The script currently only checks
CUA_DRIVER_PREBUILT_BINARY for non-empty and silences swift build output; update
the block that sets BUILT_BINARY so it first validates that
CUA_DRIVER_PREBUILT_BINARY is an absolute path and points to an existing,
readable (and executable if needed) file and if not log an "error" and exit
non-zero before any cp/sign steps; for the build path, run swift build without
discarding output (or capture stdout/stderr) and immediately check its exit
status ($?) — on failure log the captured output with process context via log
"error" and exit non-zero; ensure BUILT_BINARY is only set after successful
validation or successful build so subsequent steps never operate on an invalid
path.
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.
Summary
Validation
Summary by CodeRabbit
New Features
cleanupcommand to remove stale installer artifacts.mcpcommand with daemon-proxy mode and socket override support.Bug Fixes
Tests
Chores