feat(cua-driver): add Claude Code computer-use compatibility mode - #1424
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces Claude Code computer-use compatibility mode for cua-driver. It adds a new ChangesClaude Code Computer-Use Compatibility Feature
Sequence DiagramsequenceDiagram
participant User
participant CLI as cua-driver CLI
participant Reg as ToolRegistry
participant Server as MCP Server
participant Tool as Screenshot Tool
participant AppKit
User->>CLI: mcp --claude-code-computer-use-compat
CLI->>CLI: Parse --claude-code-computer-use-compat flag
alt Compat Mode Enabled
CLI->>Reg: Get .claudeCodeComputerUseCompat registry
CLI->>Server: Create with serverName="computer-use"<br/>and compat registry
else Default Mode
CLI->>Reg: Get .default registry
CLI->>Server: Create with serverName="cua-driver"<br/>and default registry
end
Server-->>CLI: MCP Server initialized
User->>Server: Call screenshot(pid, window_id)
alt Compat Mode
Server->>Tool: CompatTools.screenshot invoked
Tool->>AppKit: Query visible windows for pid/window_id
Tool->>AppKit: Capture single window as JPEG
Tool-->>Server: Return window-local image + metadata
else Default Mode
Server->>Tool: ScreenshotTool invoked
Tool->>AppKit: Capture specified window_id
Tool-->>Server: Return window screenshot
end
Server-->>User: Image result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 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 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}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
libs/cua-driver/Sources/CuaDriverCore/Input/MouseInput.swift (1)
96-110: ⚡ Quick winDocument
useFrontmostHIDPathbehavior in the API comment.The doc block still reads like HID posting is unconditional. With Line 110 + Line 127, HID is conditional and caller-controlled; this should be explicit to prevent incorrect assumptions about cursor movement.
Proposed doc update
- /// (`control`, `fn`); unknown names are ignored. Events are - /// posted via auth-signed `SLEventPostToPid` AND the public HID - /// tap — see the file-level doc for the rationale. + /// (`control`, `fn`); unknown names are ignored. + /// By default, frontmost targets route through `.cghidEventTap` + /// (`useFrontmostHIDPath = true`), while non-frontmost targets + /// use pid-routed delivery (`SLEventPostToPid`/`CGEvent.postToPid`).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cua-driver/Sources/CuaDriverCore/Input/MouseInput.swift` around lines 96 - 110, Update the API doc for MouseInput.click(at:toPid:button:count:modifiers:useFrontmostHIDPath:) to explicitly state that HID posting is conditional and controlled by the useFrontmostHIDPath parameter: when true the function will also post events via the frontmost HID path (which can move the global cursor), when false it will only use the auth-signed SLEventPostToPid delivery (which targets the target PID without changing the system cursor); mention the default (true) and the implications for callers who rely on cursor movement or not.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libs/cua-driver/scripts/uninstall.sh`:
- Around line 148-149: The uninstall logic only removes registrations named
"cua-driver" via the should_remove function; update should_remove (and any other
cleanup checks referring to the registration name) to also accept
"cua-computer-use" so compat MCP entries get cleaned up — e.g., change the
predicate in should_remove (and the matching logic used in the cleanup loop that
also appears around the 210-215 area) to return true when name == "cua-driver"
OR name == "cua-computer-use" or when invokes_cua_driver(server) is true.
In `@libs/cua-driver/Skills/cua-driver/SKILL.md`:
- Around line 155-156: Replace the shorthand "compat" with the full word
"compatibility" in the user-facing documentation paragraph that describes
CuaDriver tools and the `screenshot` behavior (the sentence starting "normal
CuaDriver tools and changes only `screenshot`..."); update any nearby
occurrences in SKILL.md referencing `screenshot`'s compatibility mode so the
term is consistently "compatibility" throughout the user-facing text.
In `@libs/cua-driver/Sources/CuaDriverCLI/CuaDriverCommand.swift`:
- Around line 72-74: The printed install commands for Claude/Codex interpolate
the resolvedBinaryPath() into a shell command unquoted, which breaks if the path
contains spaces; update the two print statements in the switch cases (the
"claude" and "codex" branches that currently print the command) to quote the
executable path when interpolating (e.g., wrap the binary variable in double
quotes or properly shell-escape it) so the generated command uses
"...\(binary)..." instead of an unquoted path in both the Claude and Codex print
lines.
In
`@libs/cua-driver/Sources/CuaDriverServer/ClaudeCodeComputerUseCompatTools.swift`:
- Around line 11-23: The compat session only replaces the screenshot handler but
leaves coordinate-taking tools bound to the global frame, so the downstream tool
coordinates never update; update the compat registration to also replace or wrap
all coordinate-dependent tools from ToolRegistry.default (e.g.,
click/move/region tools) so they query
ClaudeCodeComputerUseCompatSession.shared.currentActiveWindow() (use
setActiveWindow/currentActiveWindow) to compute window-local frames; ensure the
registry swap applied at the same places called out (around the other
occurrences at lines referenced) so every tool that computes coordinates uses
the activeWindow context instead of the global frame.
- Around line 64-80: The code currently force-casts numeric values with
Int32(pid) and UInt32(context.window.id) which can trap on out-of-range values;
change to checked conversions using init?(exactly:) before calling
compatWindowContext and capture.captureWindow: e.g., first convert the extracted
pid and window_id into fixed-width types with guard let pid32 = Int32(exactly:
pid) and guard let winID32 = Int32(exactly: windowID) (or the appropriate
signedness expected by compatWindowContext), return errorResult(...) on failure,
use pid32 when calling compatWindowContext(forPid:), and when passing
context.window.id into capture.captureWindow convert with guard let uiWinID =
UInt32(exactly: context.window.id) and return error on failure; update
references to use these checked variables (compatWindowContext,
capture.captureWindow, pid, windowID, context.window.id).
In `@libs/cua-driver/Sources/CuaDriverServer/Tools/ScreenshotTool.swift`:
- Around line 55-73: The code currently force-casts windowID with
UInt32(windowID) which will trap on negative or overflow values; update the
validation to perform a checked conversion using UInt32(exactly: windowID) (or
UInt32(exactly: rawWindowID) if you rename) and if that returns nil return a
CallTool.Result with isError: true and a clear message about invalid window_id
range instead of calling capture.captureWindow with an unchecked value; ensure
you update the guard that extracts arguments?["window_id"]?.intValue to use this
checked UInt32 conversion before invoking capture.captureWindow.
---
Nitpick comments:
In `@libs/cua-driver/Sources/CuaDriverCore/Input/MouseInput.swift`:
- Around line 96-110: Update the API doc for
MouseInput.click(at:toPid:button:count:modifiers:useFrontmostHIDPath:) to
explicitly state that HID posting is conditional and controlled by the
useFrontmostHIDPath parameter: when true the function will also post events via
the frontmost HID path (which can move the global cursor), when false it will
only use the auth-signed SLEventPostToPid delivery (which targets the target PID
without changing the system cursor); mention the default (true) and the
implications for callers who rely on cursor movement or not.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 768b22dd-2da9-4f9d-8a7a-3626bd6fbf1d
📒 Files selected for processing (18)
docs/content/docs/cua-driver/guide/getting-started/installation.mdxdocs/content/docs/cua-driver/guide/getting-started/integrations.mdxdocs/content/docs/cua-driver/reference/cli-reference.mdxdocs/content/docs/cua-driver/reference/mcp-tools.mdxlibs/cua-driver/README.mdlibs/cua-driver/Skills/cua-driver/README.mdlibs/cua-driver/Skills/cua-driver/SKILL.mdlibs/cua-driver/Sources/CuaDriverCLI/CallCommand.swiftlibs/cua-driver/Sources/CuaDriverCLI/ConfigCommand.swiftlibs/cua-driver/Sources/CuaDriverCLI/CuaDriverCommand.swiftlibs/cua-driver/Sources/CuaDriverCLI/Docs/CLIDocExtractor.swiftlibs/cua-driver/Sources/CuaDriverCore/Input/MouseInput.swiftlibs/cua-driver/Sources/CuaDriverServer/ClaudeCodeComputerUseCompatTools.swiftlibs/cua-driver/Sources/CuaDriverServer/CuaDriverMCPServer.swiftlibs/cua-driver/Sources/CuaDriverServer/Tools/ScreenshotTool.swiftlibs/cua-driver/scripts/install-local.shlibs/cua-driver/scripts/install.shlibs/cua-driver/scripts/uninstall.sh
| def should_remove(name, server): | ||
| return name == "cua-driver" or invokes_cua_driver(server) |
There was a problem hiding this comment.
Also remove the cua-computer-use Claude registration.
The compat install path registers under cua-computer-use, but both cleanup paths only target cua-driver by name. That leaves compat MCP entries behind after uninstall for the exact flow this PR adds.
Proposed fix
-def should_remove(name, server):
- return name == "cua-driver" or invokes_cua_driver(server)
+def should_remove(name, server):
+ return name in {"cua-driver", "cua-computer-use"} or invokes_cua_driver(server)-if command -v claude >/dev/null 2>&1; then
- for SCOPE in local project user; do
- if claude mcp remove cua-driver -s "$SCOPE" >/dev/null 2>&1; then
- log "removed Claude MCP server cua-driver from $SCOPE scope"
- fi
- done
+if command -v claude >/dev/null 2>&1; then
+ for SERVER in cua-driver cua-computer-use; do
+ for SCOPE in local project user; do
+ if claude mcp remove "$SERVER" -s "$SCOPE" >/dev/null 2>&1; then
+ log "removed Claude MCP server $SERVER from $SCOPE scope"
+ fi
+ done
+ done
else
log "claude CLI not found (skipping Claude MCP CLI cleanup)"
fiAlso applies to: 210-215
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/cua-driver/scripts/uninstall.sh` around lines 148 - 149, The uninstall
logic only removes registrations named "cua-driver" via the should_remove
function; update should_remove (and any other cleanup checks referring to the
registration name) to also accept "cua-computer-use" so compat MCP entries get
cleaned up — e.g., change the predicate in should_remove (and the matching logic
used in the cleanup loop that also appears around the 210-215 area) to return
true when name == "cua-driver" OR name == "cua-computer-use" or when
invokes_cua_driver(server) is true.
| normal CuaDriver tools and changes only `screenshot`. The compat | ||
| `screenshot` requires `pid` and `window_id`, captures only that target |
There was a problem hiding this comment.
Use “compatibility” instead of “compat” in user-facing docs.
Line 155 uses shorthand (compat) in a prominent explanatory paragraph; expanding it improves consistency with the rest of the docs.
✏️ Suggested wording tweak
- Observation: Claude Code vision flows appear to treat a screenshot MCP
- tool as the image-grounding anchor. This compatibility mode keeps the
- normal CuaDriver tools and changes only `screenshot`. The compat
+ Observation: Claude Code vision flows appear to treat a screenshot MCP
+ tool as the image-grounding anchor. This compatibility mode keeps the
+ normal CuaDriver tools and changes only `screenshot`. The compatibility
`screenshot` requires `pid` and `window_id`, captures only that target
window, and returns the window-local pixel coordinate frame.🧰 Tools
🪛 LanguageTool
[grammar] ~155-~155: Ensure spelling is correct
Context: ...ools and changes only screenshot. The compat screenshot requires pid and `window...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/cua-driver/Skills/cua-driver/SKILL.md` around lines 155 - 156, Replace
the shorthand "compat" with the full word "compatibility" in the user-facing
documentation paragraph that describes CuaDriver tools and the `screenshot`
behavior (the sentence starting "normal CuaDriver tools and changes only
`screenshot`..."); update any nearby occurrences in SKILL.md referencing
`screenshot`'s compatibility mode so the term is consistently "compatibility"
throughout the user-facing text.
| print("claude mcp add --transport stdio \(serverName) -- \(binary) \(commandArgs)") | ||
| case "codex": | ||
| print("codex mcp add cua-driver -- \(binary) mcp") | ||
| print("codex mcp add \(serverName) -- \(binary) \(commandArgs)") |
There was a problem hiding this comment.
Quote the executable path in the generated Claude/Codex install commands.
resolvedBinaryPath() can legitimately contain spaces, and these two snippets interpolate it into a shell command unquoted. In that case the pasted command is tokenized incorrectly and MCP registration fails.
Suggested fix
func run() throws {
let binary = resolvedBinaryPath()
+ let shellBinary = shellEscape(binary)
// Observed Claude Code behavior: the exact config key "computer-use"
// is reserved, so external stdio registrations use a distinct key.
let serverName = claudeCodeComputerUseCompat ? "cua-computer-use" : "cua-driver"
@@
case "claude":
- print("claude mcp add --transport stdio \(serverName) -- \(binary) \(commandArgs)")
+ print("claude mcp add --transport stdio \(serverName) -- \(shellBinary) \(commandArgs)")
case "codex":
- print("codex mcp add \(serverName) -- \(binary) \(commandArgs)")
+ print("codex mcp add \(serverName) -- \(shellBinary) \(commandArgs)")
@@
private func resolvedBinaryPath() -> String {
// `Bundle.main.executablePath` points at the physical binary
// inside the .app bundle even when invoked via a symlink. Falls
// back to argv[0] for raw `swift run` contexts.
if let path = Bundle.main.executablePath {
return path
}
return CommandLine.arguments.first ?? "cua-driver"
}
+
+ private func shellEscape(_ value: String) -> String {
+ "'\(value.replacingOccurrences(of: "'", with: "'\"'\"'"))'"
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/cua-driver/Sources/CuaDriverCLI/CuaDriverCommand.swift` around lines 72
- 74, The printed install commands for Claude/Codex interpolate the
resolvedBinaryPath() into a shell command unquoted, which breaks if the path
contains spaces; update the two print statements in the switch cases (the
"claude" and "codex" branches that currently print the command) to quote the
executable path when interpolating (e.g., wrap the binary variable in double
quotes or properly shell-escape it) so the generated command uses
"...\(binary)..." instead of an unquoted path in both the Claude and Codex print
lines.
| private actor ClaudeCodeComputerUseCompatSession { | ||
| static let shared = ClaudeCodeComputerUseCompatSession() | ||
|
|
||
| private var activeWindow: CompatWindowContext? | ||
|
|
||
| func setActiveWindow(_ context: CompatWindowContext?) { | ||
| activeWindow = context | ||
| } | ||
|
|
||
| func currentActiveWindow() -> CompatWindowContext? { | ||
| activeWindow | ||
| } | ||
| } |
There was a problem hiding this comment.
The compat screenshot never changes downstream tool coordinates.
activeWindow is only written here, and this registry swaps in only the screenshot handler. Since the coordinate-taking tools still come from ToolRegistry.default, this mode does not actually establish the window-local frame promised by the tool description.
Also applies to: 83-88, 112-119
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@libs/cua-driver/Sources/CuaDriverServer/ClaudeCodeComputerUseCompatTools.swift`
around lines 11 - 23, The compat session only replaces the screenshot handler
but leaves coordinate-taking tools bound to the global frame, so the downstream
tool coordinates never update; update the compat registration to also replace or
wrap all coordinate-dependent tools from ToolRegistry.default (e.g.,
click/move/region tools) so they query
ClaudeCodeComputerUseCompatSession.shared.currentActiveWindow() (use
setActiveWindow/currentActiveWindow) to compute window-local frames; ensure the
registry swap applied at the same places called out (around the other
occurrences at lines referenced) so every tool that computes coordinates uses
the activeWindow context instead of the global frame.
There was a problem hiding this comment.
Verified against the current tool implementations. I did not wrap the coordinate-taking tools because the normal CuaDriver pixel tools already accept window-local screenshot coordinates and convert through WindowCoordinateSpace using pid plus window_id when supplied. In compat mode the screenshot shim requires pid and window_id, and the docs now tell Claude Code users to pass that same target window into the normal CuaDriver tools. Wrapping every coordinate tool here would duplicate the existing coordinate conversion layer and broaden the compat mode beyond the intended screenshot-only shim.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check WindowCoordinateSpace and how coordinate tools use pid + window_id
rg -n "WindowCoordinateSpace" --type swift -C3
echo "---"
rg -n "window_id" libs/cua-driver/Sources/CuaDriverServer/Tools/ -C3
echo "---"
rg -n "pid" libs/cua-driver/Sources/CuaDriverServer/Tools/ -C3 --type swift
Length of output: 312101
@coderabbit-user Thanks for the clarification — I re-checked the code paths and your reasoning lines up with what’s implemented.
What I verified
- Pixel tools already convert window-local pixels using pid + window_id:
- click: WindowCoordinateSpace.screenPoint(fromImagePixel:..., forPid:, windowId:) when window_id is present; otherwise pid-only fallback to the pid’s frontmost window.
- double_click/right_click/drag: same pattern — windowId-aware mapping when provided; pid-only frontmost fallback when omitted.
- Element-indexed paths already require window_id and are scoped per (pid, window_id).
- Image resizing coordination: GetWindowStateTool sets ImageResizeRegistry’s ratio for the pid when it resizes the screenshot, and click/drag use that recorded ratio to scale x,y back up.
Net: If Claude Code calls the normal click/move/drag tools with the same pid + window_id used for the compat screenshot, the mapping lands in the correct window-local frame without additional wrappers. So I’ll withdraw the earlier “wrap everything” suggestion.
One small follow-up to consider
- If the compat screenshot is ever downscaled (e.g., to max_image_dimension) and only records scaleFactor in the private ClaudeCodeComputerUseCompatSession, pixel tools won’t see it because they read ImageResizeRegistry.ratio(forPid:). Two easy options:
- Emit screenshots at native scale in compat mode; or
- In the compat screenshot handler, also call ImageResizeRegistry.shared.setRatio(scaleFactor, forPid: pid) so existing click/drag scaling logic continues to work without extra wrappers.
If you’re already returning native-scale JPEGs in compat mode, then no change is needed. Otherwise, the single-line ImageResizeRegistry update in the compat screenshot path would close that gap and make the session actor unnecessary. If you’d like, I can draft that tiny patch.
| guard let pid = arguments?["pid"]?.intValue else { | ||
| return errorResult("Missing required integer field `pid`.") | ||
| } | ||
| guard let windowID = arguments?["window_id"]?.intValue else { | ||
| return errorResult("Missing required integer field `window_id`.") | ||
| } | ||
| guard let context = compatWindowContext( | ||
| forPid: Int32(pid), | ||
| windowID: windowID | ||
| ) else { | ||
| return errorResult( | ||
| "No visible layer-0 window \(windowID) found for pid \(pid). Use `list_windows` to choose an on-screen target window." | ||
| ) | ||
| } | ||
| let shot = try await capture.captureWindow( | ||
| windowID: UInt32(context.window.id), | ||
| format: .jpeg, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Swift, do Int32(someInt)andUInt32(someInt)trap whensomeInt is outside the destination type's range? What checked conversion API is recommended for request validation?
💡 Result:
Yes, in Swift, Int32(someInt) and UInt32(someInt) trap (crash at runtime) when someInt is outside the destination type's representable range. This is the range-checked behavior of the default init(:) initializer provided by the BinaryInteger protocol, which all fixed-width integer types like Int32 and UInt32 conform to. Apple documentation explicitly states that using init(:) triggers a runtime error if the value cannot be represented exactly in the destination type. For request validation, where you need to safely handle potentially out-of-range values (e.g., from untrusted input like JSON or HTTP parameters), the recommended checked conversion API is the failable initializer init?(exactly:). It returns nil if the value is out of range, allowing you to gracefully handle invalid input without trapping. Other options like init(clamping:) or init(truncatingIfNeeded:) exist for specific behaviors, but init?(exactly:) is preferred for validation as it precisely checks representability without modifying the value.
Citations:
- 1: https://numerics.diploid.ca/integers-part-1.html
- 2: https://stackoverflow.com/questions/32793460/can-i-cast-int64-directly-into-int
- 3: https://developer.apple.com/documentation/swift/binaryinteger
- 4: https://swiftdoc.org/v2.1/type/int32/
- 5: https://developer.apple.com/documentation/swift/int32
🏁 Script executed:
# Find the file and check if it exists
fd "ClaudeCodeComputerUseCompatTools.swift"Repository: trycua/cua
Length of output: 133
🏁 Script executed:
# Read the specific lines mentioned in the review
if [ -f "libs/cua-driver/Sources/CuaDriverServer/ClaudeCodeComputerUseCompatTools.swift" ]; then
sed -n '60,85p' "libs/cua-driver/Sources/CuaDriverServer/ClaudeCodeComputerUseCompatTools.swift"
fiRepository: trycua/cua
Length of output: 1230
🏁 Script executed:
# Check broader context around those lines to understand the function signature
if [ -f "libs/cua-driver/Sources/CuaDriverServer/ClaudeCodeComputerUseCompatTools.swift" ]; then
sed -n '40,95p' "libs/cua-driver/Sources/CuaDriverServer/ClaudeCodeComputerUseCompatTools.swift"
fiRepository: trycua/cua
Length of output: 2685
🏁 Script executed:
# Find the definition of intValue property (likely a JSON value extension)
rg "intValue" libs/cua-driver/Sources/CuaDriverServer/ -A 2 -B 2Repository: trycua/cua
Length of output: 24289
🏁 Script executed:
# Find the compatWindowContext function signature
rg "func compatWindowContext" libs/cua-driver/Sources/CuaDriverServer/ -A 5Repository: trycua/cua
Length of output: 712
🏁 Script executed:
# Check the type of context.window.id by looking at the window/context definition
rg "struct.*Window|class.*Window" libs/cua-driver/Sources/CuaDriverServer/ -A 10 | head -50Repository: trycua/cua
Length of output: 2313
Guard pid and window_id before fixed-width casts.
Int32(pid) and UInt32(context.window.id) trap on out-of-range input, turning a malformed MCP request into a process abort instead of a normal tool error. Use init?(exactly:) checked conversions, which is already the pattern used throughout the codebase (ScrollTool, TypeTextTool, PageTool, ListWindowsTool).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@libs/cua-driver/Sources/CuaDriverServer/ClaudeCodeComputerUseCompatTools.swift`
around lines 64 - 80, The code currently force-casts numeric values with
Int32(pid) and UInt32(context.window.id) which can trap on out-of-range values;
change to checked conversions using init?(exactly:) before calling
compatWindowContext and capture.captureWindow: e.g., first convert the extracted
pid and window_id into fixed-width types with guard let pid32 = Int32(exactly:
pid) and guard let winID32 = Int32(exactly: windowID) (or the appropriate
signedness expected by compatWindowContext), return errorResult(...) on failure,
use pid32 when calling compatWindowContext(forPid:), and when passing
context.window.id into capture.captureWindow convert with guard let uiWinID =
UInt32(exactly: context.window.id) and return error on failure; update
references to use these checked variables (compatWindowContext,
capture.captureWindow, pid, windowID, context.window.id).
| guard let windowID = arguments?["window_id"]?.intValue else { | ||
| return CallTool.Result( | ||
| content: [ | ||
| .text( | ||
| text: "Missing required `window_id`. Use `list_windows` first, then call `screenshot` for one window.", | ||
| annotations: nil, | ||
| _meta: nil | ||
| ) | ||
| ], | ||
| isError: true | ||
| ) | ||
| } | ||
|
|
||
| do { | ||
| let shot: Screenshot | ||
| if let windowID { | ||
| shot = try await capture.captureWindow( | ||
| windowID: UInt32(windowID), | ||
| format: format, | ||
| quality: quality | ||
| ) | ||
| } else { | ||
| shot = try await capture.captureMainDisplay( | ||
| format: format, | ||
| quality: quality | ||
| ) | ||
| } | ||
| let shot = try await capture.captureWindow( | ||
| windowID: UInt32(windowID), | ||
| format: format, | ||
| quality: quality | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Swift, does UInt32(someInt)trap whensomeIntis negative or outsideUInt32's range? What checked conversion API should be used instead?
💡 Result:
Yes, UInt32(someInt) traps (causes a runtime error) if someInt is negative or outside UInt32's representable range (0...UInt32.max). Use the failable initializer UInt32(exactly: someInt) for checked conversion, which returns nil if the value cannot be exactly represented. Alternatively, UInt32(truncatingIfNeeded: someInt) truncates without trapping, or UInt32(clamping: someInt) clamps to the range.
Citations:
- 1: https://developer.apple.com/documentation/swift/binaryinteger
- 2: https://sosumi.ai/documentation/swift/binaryinteger/init(_:)-8gmdl
- 3: https://numerics.diploid.ca/integers-part-1.html
- 4: https://developer.apple.com/documentation/swift/uint32/init(_:)-2w61y?changes=l_8
Validate window_id range before casting to prevent server crash.
UInt32(windowID) traps if the value is negative or exceeds UInt32.max, causing a runtime crash instead of returning isError: true. Use UInt32(exactly: rawWindowID) for a checked conversion that returns nil on invalid range, allowing graceful error handling.
Proposed fix
- guard let windowID = arguments?["window_id"]?.intValue else {
+ guard
+ let rawWindowID = arguments?["window_id"]?.intValue,
+ let windowID = UInt32(exactly: rawWindowID)
+ else {
return CallTool.Result(
content: [
.text(
- text: "Missing required `window_id`. Use `list_windows` first, then call `screenshot` for one window.",
+ text: "Missing or invalid `window_id`. Use `list_windows` first, then pass a valid window id.",
annotations: nil,
_meta: nil
)
],
isError: true
)
}
@@
- let shot = try await capture.captureWindow(
- windowID: UInt32(windowID),
+ let shot = try await capture.captureWindow(
+ windowID: windowID,
format: format,
quality: quality
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/cua-driver/Sources/CuaDriverServer/Tools/ScreenshotTool.swift` around
lines 55 - 73, The code currently force-casts windowID with UInt32(windowID)
which will trap on negative or overflow values; update the validation to perform
a checked conversion using UInt32(exactly: windowID) (or UInt32(exactly:
rawWindowID) if you rename) and if that returns nil return a CallTool.Result
with isError: true and a clear message about invalid window_id range instead of
calling capture.captureWindow with an unchecked value; ensure you update the
guard that extracts arguments?["window_id"]?.intValue to use this checked UInt32
conversion before invoking capture.captureWindow.
1 participant
Summary
Verification
Summary by CodeRabbit
Release Notes
New Features
--claude-code-computer-use-compatflag for MCP registrationDocumentation