feat(cua-driver): WKWebView/Tauri AX fallback for get_text and query_dom - #1389
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
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 a comprehensive JavaScript execution infrastructure for the CUA driver, adding support for executing JavaScript across multiple browser and app types (Chrome/Safari via AppleScript, Electron via CDP and SIGUSR1, WebKit via TCP inspector). It includes new backend modules, a unified PageTool for browser page operations, accessibility tree parsing without JavaScript, and integration tests validating the functionality. Changes
Sequence DiagramssequenceDiagram
participant Client
participant PageTool
participant AppDetector as App Detection<br/>(WebInspectorXPC)
participant AXReader as AX Reader
participant BrowserJS
participant ElectronJS
participant CDPClient
participant Process as Target App
Client->>PageTool: page(action, pid, window_id)
PageTool->>AppDetector: Detect app type<br/>(isElectron, isWKWebViewApp)
AppDetector->>Process: Check bundle, frameworks
Process-->>AppDetector: App type result
alt Electron App
PageTool->>ElectronJS: execute(javascript, pid)
ElectronJS->>Process: SIGUSR1 (activate V8)
ElectronJS->>CDPClient: Evaluate on inspector port
CDPClient->>Process: WebSocket CDP request
Process-->>CDPClient: Result
CDPClient-->>ElectronJS: Return value
ElectronJS-->>PageTool: JavaScript output
else Chrome/Safari
PageTool->>BrowserJS: execute(javascript, bundleId, windowId)
BrowserJS->>Process: Build/execute AppleScript
Process-->>BrowserJS: JavaScript result
BrowserJS-->>PageTool: Return value
else WKWebView/Tauri
PageTool->>AXReader: Extract text or query
AXReader->>PageTool: AX tree results
end
PageTool-->>Client: Result (data/error)
sequenceDiagram
participant Script as AppleScript
participant osascript as /usr/bin/osascript
participant Browser as Browser Process
participant Tab as Active Tab
Script->>osascript: Execute browser-specific JS script
osascript->>Browser: Activate & inject script
Browser->>Tab: Run JavaScript in context
Tab-->>Browser: Evaluate result
Browser-->>osascript: Result string (or error)
osascript-->>Script: stdout/stderr mapping
Script->>Script: Parse errors<br/>(javascriptNotEnabled,<br/>executionFailed)
sequenceDiagram
participant PageTool
participant snapshot as AppStateRegistry<br/>.engine.snapshot
participant AXTree as Accessibility<br/>Tree Markdown
participant Parser as AXPageReader
PageTool->>snapshot: Fetch current AX tree
snapshot-->>AXTree: treeMarkdown string
alt get_text action
PageTool->>Parser: extractText(from:)
Parser->>AXTree: Parse each line
Parser->>Parser: Filter roles<br/>(headings, static text,<br/>interactive values)
Parser->>Parser: De-duplicate<br/>consecutive lines
Parser-->>PageTool: Visible text string
else query_dom action
PageTool->>Parser: query(selector, from:)
Parser->>Parser: Map CSS selector<br/>to AX role
Parser->>AXTree: Scan for role matches
Parser->>Parser: Extract title, value,<br/>index, description
Parser-->>PageTool: [Element] array
end
PageTool-->>Client: Result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
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 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: 11
🧹 Nitpick comments (12)
libs/cua-driver/Sources/CuaDriverCore/Permissions/Permissions.swift (1)
25-28: Update the stale doc comment to match the new probe API.The doc comment on
currentStatus()still describes the probe as usingSCShareableContent.current, but the implementation now usesexcludingDesktopWindows(false, onScreenWindowsOnly: true). Worth updating so readers don't get misled, and to record the rationale (avoiding the multi-second hang from off-screen window enumeration, per#1371).📝 Proposed doc update
/// Accurate TCC status for both grants. /// /// Accessibility uses `AXIsProcessTrusted()` — reliable. /// - /// Screen Recording does a real probe via `SCShareableContent.current` - /// rather than `CGPreflightScreenCaptureAccess()` — the latter returns - /// false negatives for subprocess-launched apps, even when the grant is - /// active. The probe costs ~100-300ms but returns the truth. + /// Screen Recording does a real probe via + /// `SCShareableContent.excludingDesktopWindows(_:onScreenWindowsOnly:)` + /// rather than `CGPreflightScreenCaptureAccess()` — the latter returns + /// false negatives for subprocess-launched apps, even when the grant is + /// active. We restrict to on-screen windows to avoid multi-second hangs + /// when the window server has many off-screen entries (see `#1371`). + /// The probe costs ~100-300ms but returns the truth.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cua-driver/Sources/CuaDriverCore/Permissions/Permissions.swift` around lines 25 - 28, The doc comment for currentStatus() is stale—update it to describe that the probe now uses the CGWindowList API with excludingDesktopWindows(false, onScreenWindowsOnly: true) instead of SCShareableContent.current; mention the rationale that this avoids the multi-second hang caused by enumerating off-screen windows (per `#1371`) while still accurately probing screen-recording permission, and adjust wording to note the ~100–300ms probe cost if still applicable.libs/cua-driver/Sources/CuaDriverCore/Browser/CDPClient.swift (2)
83-87: Force-unwrap ofString(data:encoding:)is safe but an avoidable smell.
JSONSerialization.data(withJSONObject:)always produces valid UTF-8, so the!won't trigger in practice — but it's the kind of construct that makes audits flag the file. A simpleguard letwith a thrownError.connectionFailed("payload encoding")keeps the contract explicit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cua-driver/Sources/CuaDriverCore/Browser/CDPClient.swift` around lines 83 - 87, Replace the force-unwrap when creating payloadStr: instead of using String(data: try JSONSerialization.data(withJSONObject: payload), encoding: .utf8)!, use a guard let on String(data: ..., encoding: .utf8) to safely unwrap and, on failure, throw Error.connectionFailed("payload encoding"); update the code at the payloadStr creation site in CDPClient.swift (look for the payloadStr variable and the JSONSerialization.data(withJSONObject:) call) so the method's contract is explicit and avoids the force-unwrap smell.
88-112: No bounded timeout on the WebSocket evaluate path.
evaluatehas no explicit deadline aroundws.send/ws.receive. If the inspector hangs (target paused at a breakpoint, network blip, frozen renderer), the continuation never resumes and the calling task is wedged indefinitely. Inspector hangs are real failure modes (Electron page targets paused ondebugger;, Tauri/WebKit targets stuck during navigation).Wrap the await in
withTimeout(or useTask.sleep+ race) and surface asError.evaluationFailed("timeout after Ns").🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cua-driver/Sources/CuaDriverCore/Browser/CDPClient.swift` around lines 88 - 112, The evaluate path currently awaits a WebSocket send/receive inside withCheckedThrowingContinuation with no deadline, so if the inspector hangs the continuation is never resumed; modify the evaluate implementation (the async method that creates the URLSession.webSocketTask, calls ws.send and ws.receive and resumes the continuation) to impose a bounded timeout by racing the continuation against a timeout Task (or using your withTimeout helper), and if the timeout wins call continuation.resume(throwing: Error.evaluationFailed("timeout after Ns")) and cancel the webSocketTask; ensure all existing resume paths (ws send error, receive failure/success, and the new timeout branch) still only resume once and that ws.cancel() is invoked when timing out.libs/cua-driver/Sources/CuaDriverCore/Browser/WebInspectorXPC.swift (2)
45-48:bundlePath.contains("tauri")is a fragile string heuristic.Any app whose install path or bundle name happens to include "tauri" (case-insensitive — comment quotes only
tauriwhile the path has been lowercased, so e.g./Applications/Tauri Tools/SomeNonTauriApp.appmatches) gets force-classified as WKWebView and short-circuits the WebKit-linkage check below. The comment itself acknowledges "no good heuristic beyond bundle name."In practice the executable WebKit-linkage probe (
otool -L) is the reliable signal, so thetaurishortcut mainly serves to skip anotoolcall. Either remove it and rely on the linkage check, or tighten it (e.g., look for aContents/MacOS/<name>binary linked againstlibwry/taurisymbols) to avoid surprising false positives.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cua-driver/Sources/CuaDriverCore/Browser/WebInspectorXPC.swift` around lines 45 - 48, The current fragile heuristic using bundlePath.contains("tauri") in WebInspectorXPC.swift incorrectly classifies many apps as Tauri; remove this short-circuit and rely on the existing WebKit-linkage probe (the otool -L check) for accurate detection, or if you must keep a fast path, tighten it by resolving the app bundle's Contents/MacOS/<executable> and checking that binary's linkage/symbols for known Tauri/libwry identifiers before returning true (refer to bundleURL, bundlePath.contains("tauri") and the WebKit-linkage probe logic to locate and update the decision point).
55-69:/usr/bin/otool -Lruns synchronously on everyisWKWebViewAppcall — consider caching by bundle URL.
PageToolcalls this on everyget_text/query_dom/execute_javascriptto gate the dispatch. Each call forksotool, which is non-trivial overhead and adds end-to-end latency before the snapshot work even starts. The result is stable for the lifetime of an app's executable, so caching keyed by(bundleURL, executableURL)(or simply by bundle id) would eliminate the repeat cost.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cua-driver/Sources/CuaDriverCore/Browser/WebInspectorXPC.swift` around lines 55 - 69, The isWKWebViewApp flow currently calls isLinkedToWebKit (which runs runProcess("/usr/bin/otool", ...)) on every check, so add a static cache (e.g., [URL: Bool] or [String: Bool] keyed by bundleURL or bundle identifier) inside WebInspectorXPC to remember results per app; update isWKWebViewApp to first consult this cache and return the cached Bool if present, otherwise call isLinkedToWebKit, store the result in the cache, and return it; ensure thread-safety around the cache (use a serial DispatchQueue or lock) and keep references keyed by bundle/executable URL (or bundle id) to avoid rerunning the expensive otool subprocess repeatedly.libs/cua-driver/Tests/integration/test_webkit_js.py (2)
178-183: Hardcodedtime.sleep(3)after launch is brittle.3 s is a magic number that will be flaky on CI under load and slow on first launch (cold cache, gatekeeper scan). Consider polling
_find_conductor_pidwith a short timeout instead, so the test fails fast when the launch genuinely doesn't complete and isn't slowed unnecessarily on warm runs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cua-driver/Tests/integration/test_webkit_js.py` around lines 178 - 183, Replace the brittle fixed sleep after calling "launch_app" with a short polling loop that repeatedly calls _find_conductor_pid until it returns a PID or a configurable timeout elapses; specifically, after cls.client.call("launch_app", ...) remove time.sleep(3) and instead poll _find_conductor_pid(cls.client) with a small interval (e.g. 0.1–0.5s) and a total timeout (e.g. a few seconds) and set cls.pid when found, raising/asserting a clear failure if the timeout elapses so the test fails fast on genuine launch failures.
137-148:if window_id:treats 0 as missing.
CGWindowIDis unsigned and the kernel can in principle return 0 (it'skCGNullWindowID, but parsingid=0fromlist_windowstext would still hit this). More importantly the same idiom is used twice (here and in_get_window_idinner loop) — usingis not Nonemakes the intent explicit and avoids the falsy-zero gotcha if the id ever legitimately surfaces.- if window_id: + if window_id is not None: break🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cua-driver/Tests/integration/test_webkit_js.py` around lines 137 - 148, The code incorrectly checks the parsed window_id with a truthy test which treats 0 as missing; update the conditionals that check window_id (both the outer check here and the inner loop in _get_window_id) to use explicit "is not None" comparisons so 0 is treated as a valid CGWindowID and the intent is clear; ensure you only call self.skipTest("No Chrome window found") when window_id is None.libs/cua-driver/Sources/CuaDriverCore/Browser/ElectronJS.swift (1)
189-206:runProcessdoesn't drain stderr; stdout read is also lazy.
proc.standardError = Pipe()is set but never read — for a verbose process this could fill the kernel pipe buffer (~16-64 KB) and block the child indefinitely. Forlsofcalls used here, output is small enough this is unlikely to bite, but it's a footgun if this helper is reused.A small refactor to read both pipes (or redirect stderr to
/dev/nullsince callers don't consume it) would harden it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cua-driver/Sources/CuaDriverCore/Browser/ElectronJS.swift` around lines 189 - 206, The runProcess helper (function runProcess) sets proc.standardError = Pipe() but never reads it and only reads stdout lazily in the terminationHandler, which can deadlock if stderr fills; fix by creating a separate Pipe for stderr (e.g., let errPipe = Pipe()), read both pipe.fileHandleForReading and errPipe.fileHandleForReading (collect dataToEndOfFile) concurrently before/respecting termination, then combine or discard stderr (or redirect to /dev/null) as callers expect, and ensure continuation.resume is called exactly once with the aggregated stdout (and optionally include stderr in the returned string or log it) while catching proc.run() errors and cleaning up handlers (update references to proc, pipe, standardError, terminationHandler, and runProcess).libs/cua-driver/Sources/CuaDriverCore/Browser/WebKitJS.swift (1)
9-11: Doc-comment "near-instant" fall-through is closer to ~1.5 s.With 3 ports at 0.5 s timeout each, an all-miss probe takes up to 1.5 s — not "near-instant". Worth noting since this runs on every
pageaction dispatch on macOS where the WebKit TCP inspector is never present.If
WebKitJS.isAvailable()is on the hot path, consider caching a negative result for the lifetime of the process or short-circuiting based on platform/isWKWebViewAppdetection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cua-driver/Sources/CuaDriverCore/Browser/WebKitJS.swift` around lines 9 - 11, Update the doc-comment to stop saying "near-instant" and state the aggregate timeout (~1.5s for 3 ports at 0.5s each) and that probes can be slow on macOS; then change the hot-path probe logic inside WebKitJS.isAvailable() to avoid repeated full probes by caching a negative result for the process lifetime (e.g. a static/cached Optional Bool used on subsequent calls) and/or short-circuit early using platform or isWKWebViewApp detection before performing the per-port timeouts so repeated page dispatches don't incur the 1.5s cost.libs/cua-driver/Sources/CuaDriverCore/Browser/BrowserJS.swift (1)
198-204: Substring matching on error messages is potentially locale-fragile, but no documented error code alternative exists.The check
err.contains("turned off")relies on osascript's stderr output, which could theoretically be localized. However, web search found no documented numeric error code for the "Allow JavaScript from Apple Events" disabled condition—Chrome returns a descriptive English message, and no standard numeric error codes are tied to this specific setting. The suggestion to checkterminationStatusor match on error codes like(-2740)/(-1753)lacks supporting documentation.If this becomes a problem in practice (i.e., if the error message is actually localized on non-English systems), consider detecting this condition more robustly—for example, by checking for specific app bundles before attempting execution, or by improving error message detection to handle variants. For now, this remains a known limitation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cua-driver/Sources/CuaDriverCore/Browser/BrowserJS.swift` around lines 198 - 204, The current error detection uses fragile substring matching on err to decide when to throw Error.javascriptNotEnabled(appName, bundleId); change the check so it normalizes err (lowercase and trimmed) and does a case-insensitive contains match (e.g., on "turned off" and "applescript is turned off") to reduce locale/format fragility, and add a short TODO comment near the p.terminationStatus / continuation.resume block noting the known limitation and recommending future improvements (pre-checking target app bundle permissions or a more robust AppleEvent/TCC check) if localization issues arise.libs/cua-driver/Tests/integration/test_browser_js.py (1)
283-284: Fragile regex with unclear intent.
r"elements.*turn"is opaque — it presumably matches AX tree summary text like "N elements... return" but the intent isn't documented and the pattern is brittle to copy changes. Consider asserting on a specific, stable AX tree marker (e.g., the section header thatget_window_statealways emits) and add a comment describing what content is expected.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cua-driver/Tests/integration/test_browser_js.py` around lines 283 - 284, The current fragile regex self.assertRegex(text, r"elements.*turn", re.IGNORECASE) in test_browser_js.py is opaque and brittle; replace it with an assertion that targets a stable AX tree marker emitted by get_window_state (for example the fixed section header string that get_window_state always includes, e.g., "AX Tree" or the exact header used in get_window_state), and add a one-line comment above the assertion describing the expected AX tree content and why that header is stable; update the assertion to use self.assertIn(stable_header, text) or a precise regex matching that header to make the test stable and readable.libs/cua-driver/Sources/CuaDriverCore/Browser/AXPageReader.swift (1)
95-137: Unrecognized tag selectors silently match everything.
cssToAXRolesreturns[]for*, empty, class/id-only, and for unrecognized tags (default case). Inquery, empty roles becomesmatchAll = true, so a typo likequery(selector: "xyzzy", …)returns the entire AX tree instead of zero elements. CSS-wise, an unknown tag would normally match nothing.Consider distinguishing "explicit wildcard" from "unrecognized" so that callers can get an empty result for typos/unsupported selectors.
♻️ Possible refactor
private static func cssToAXRoles(_ selector: String) -> Set<String> { - // Strip pseudo-classes, attribute selectors, combinators for simplicity. + // Strip pseudo-classes, attribute selectors, combinators for simplicity. let cleaned = selector .components(separatedBy: CharacterSet(charactersIn: ":>+~[")) .first? .trimmingCharacters(in: .whitespaces) ?? selector - // If it's purely a class (.foo) or id (`#foo`) selector we can't map it. - if cleaned.hasPrefix(".") || cleaned.hasPrefix("#") || cleaned == "*" || cleaned.isEmpty { - return [] - } + // Wildcard / class / id only → match everything (caller convention). + if cleaned == "*" || cleaned.isEmpty + || cleaned.hasPrefix(".") || cleaned.hasPrefix("#") { + return [] + } ... - default: return [] + default: return ["__AX_NO_MATCH__"] // sentinel that no role will equal } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cua-driver/Sources/CuaDriverCore/Browser/AXPageReader.swift` around lines 95 - 137, cssToAXRoles currently conflates explicit wildcards (like "*" or empty/class/id-only selectors) with unrecognized tag names by returning an empty Set, causing the query(...) caller to treat typos as "match all". Change cssToAXRoles to distinguish these cases: have it return an Optional Set (or a small enum) where nil (or a MatchAll sentinel) means explicit wildcard/class-or-id-only (preserve existing match-all behavior) but return an actual empty Set for truly unrecognized tags so callers get zero matches; then update query(...) to treat only the nil/MatchAll sentinel as matchAll and treat an empty Set as matchNone. Ensure references: cssToAXRoles and query are adjusted accordingly.
🤖 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/Skills/cua-driver/SKILL.md`:
- Around line 773-775: The fenced code block containing get_window_state({pid,
window_id, javascript: "document.title"}) is missing a language tag which
triggers markdownlint MD040; update the triple-backtick opener to include a
language (e.g., text or javascript) so it becomes ```text (or ```javascript) and
keep the block content unchanged to provide proper syntax highlighting and lint
compliance in SKILL.md.
In `@libs/cua-driver/Sources/CuaDriverCore/Browser/BrowserJS.swift`:
- Around line 226-242: The current 5s polling loop using MainActor.run over
NSWorkspace.shared.runningApplications (filtering by bundleId) can exit while
the browser is still running and then proceed to JSONSerialization.data, leading
to a race; change the logic so that after calling app.terminate() you either (a)
if any apps still exist after the deadline, throw an enable-failed error (e.g.,
throw .enableFailed(...)) to fail fast, or (b) extend the wait and for any
remaining apps call app.forceTerminate() and then wait until
NSWorkspace.shared.runningApplications no longer contains bundleId before
proceeding to write Preferences; ensure you reference the same bundleId, use
app.terminate()/app.forceTerminate(), and only continue to
JSONSerialization.data once no runningApplications contain the bundleId.
- Around line 221-305: The enableJavaScriptAppleEvents flow incorrectly treats
Safari as unsupported because profilesDirectory(for:) returns nil; update
enableJavaScriptAppleEvents to handle bundleId == "com.apple.Safari" specially
(or surface a clear Safari-specific error) by either: (A) implementing the
Safari path — run the appropriate defaults write for key
"AllowJavaScriptFromAppleEvents" and ensure the Develop menu requirement is
documented before returning, or (B) immediately throw a descriptive Error (e.g.
in enableJavaScriptAppleEvents when bundleId == "com.apple.Safari") that tells
callers Safari must be enabled via the Develop menu and the defaults write
command; also update javascriptNotEnabledMessage to include the Safari-specific
manual steps when bundleId == "com.apple.Safari". Use the symbols
enableJavaScriptAppleEvents(bundleId:), profilesDirectory(for:),
javascriptNotEnabledMessage and the Error enum to locate and implement the
change.
In `@libs/cua-driver/Sources/CuaDriverCore/Browser/CDPClient.swift`:
- Around line 88-111: The receive path may consume an unsolicited event frame
because it reads exactly one frame and passes it to parseResult; change
CDPClient's request/response flow to loop-reading ws.receive until a frame with
a matching "id" is found (or a bounded timeout occurs) instead of assuming the
first frame is the response. Implement a helper (e.g.,
receiveMatchingResponse(webSocket: URLSessionWebSocketTask, expectedId: Int,
timeout: TimeInterval)) that receives frames, parses each string payload to
JSON, inspects obj["id"] and returns the string for the matching id (or throws
Error.connectionFailed on timeout/parse failure), then call parseResult only
with that matched frame; ensure the original continuation usage in
withCheckedThrowingContinuation remains but uses this helper instead of a single
ws.receive.
In `@libs/cua-driver/Sources/CuaDriverCore/Browser/ElectronJS.swift`:
- Around line 96-122: The polling loop currently calls scanInspectorPorts() on
every iteration which can massively exceed the documented 2s budget; change the
flow so the loop only polls listeningPorts(pid:) and checks isInspectorPort(_:)
for newPorts for up to 10 iterations, and if inspectorPort is still nil after
the loop run scanInspectorPorts() exactly once as a final fallback (using its
result to set inspectorPort before the guard/throw). Update references: remove
the scanInspectorPorts() call from inside the for _ in 0..<10 loop and add a
single call to scanInspectorPorts() after the loop that assigns inspectorPort if
non-nil.
In `@libs/cua-driver/Sources/CuaDriverCore/Browser/WebInspectorXPC.swift`:
- Around line 34-61: The method isWKWebViewApp accesses
NSWorkspace.shared.runningApplications from a non-main-actor context which is
unsafe; update the function to be main-actor safe by either (A) annotating
isWKWebViewApp with `@MainActor` and making callers await the new async function,
or (B) keep it synchronous but wrap the NSWorkspace access in await
MainActor.run { ... } to fetch runningApplications and app.bundleURL before
continuing; ensure the check that uses isLinkedToWebKit(executableURL:) is
invoked only after the MainActor-sourced values are captured so you do not
access AppKit objects off the main actor.
In `@libs/cua-driver/Sources/CuaDriverServer/Tools/PageTool.swift`:
- Around line 200-220: Attribute names in attrs are interpolated directly into
attrJS without escaping, which can break the generated JS or allow injection;
update the attrJS construction to run each attribute name through the existing
jsonString helper (the same way selector is escaped) before joining them so that
attrJS uses the safely-escaped values. Specifically, transform the attrs ->
attrJS mapping to use jsonString(...) for each element (referencing attrs,
attrJS and jsonString in PageTool.swift) so attribute names containing quotes,
backslashes, or newlines are properly escaped.
- Around line 165-231: For the WKWebView/Tauri code paths in the get_text and
query_dom cases, stop falling through to executeJS when
WebInspectorXPC.isWKWebViewApp(pid:) is true; if axGetText(...) or
axQueryDom(...) returns nil because the snapshot succeeded but had no matches,
treat that as a valid empty result (return okResult with "## Page text (via AX
tree)\n\n\(axText)" for get_text and for query_dom return okResult with an empty
JSON array e.g. "[]"); update the query_dom branch to coalesce axQueryDom(...)
to "[]" (or change axQueryDom to return "[]" when snapshot succeeded with zero
matches and only return nil for true snapshot failures) and ensure executeJS is
only attempted when not a WKWebView app.
- Around line 165-199: The get_text and query_dom branches incorrectly route
Safari to the AX-tree fallback because they test
WebInspectorXPC.isWKWebViewApp(pid:) before checking JavaScript support; update
both cases to first call BrowserJS.supports(bundleId:) (same guard used by
executeJS) and, only if JS is unsupported, then fall back to
WebInspectorXPC.isWKWebViewApp(pid:) and the axGetText/axQueryDom paths; keep
executeJS/axGetText/axQueryDom calls and error handling unchanged so
WKWebView/Tauri apps without JS support still use the AX-tree fallback.
In `@libs/cua-driver/Tests/integration/test_browser_js.py`:
- Around line 69-79: The prefs write is non-atomic and can corrupt Chrome if
interrupted; update the block that opens prefs_path to write to a temp file in
the same directory (use tempfile.NamedTemporaryFile or create prefs_path +
".tmp" in os.path.dirname(prefs_path)), json.dump the data to that temp file,
fsync and close it, then atomically replace the original with
os.replace(temp_path, prefs_path); ensure you only replace after successful dump
and consider preserving permissions if needed.
In `@libs/cua-driver/Tests/integration/test_webkit_js.py`:
- Around line 242-277: The tests test_query_dom_buttons_via_ax_tree and
test_query_dom_links_via_ax_tree only assert the output is not a crash but don't
verify the AX-fallback actually returned data; change each test to assert the
call result is not an error (use _is_error(result) is False) and that the
payload contains at least one element (non-empty list/field returned by the
query_dom response), and if the page legitimately has no matching elements on
first launch then call self.skipTest with an explanatory message; locate the
checks inside those two test methods (they already build result via
self.client.call("page", {...}) and compute text = _tool_text(result)) and add
the _is_error and non-empty payload assertions immediately after obtaining
result/text, keeping the existing crash assertions as secondary safeguards.
---
Nitpick comments:
In `@libs/cua-driver/Sources/CuaDriverCore/Browser/AXPageReader.swift`:
- Around line 95-137: cssToAXRoles currently conflates explicit wildcards (like
"*" or empty/class/id-only selectors) with unrecognized tag names by returning
an empty Set, causing the query(...) caller to treat typos as "match all".
Change cssToAXRoles to distinguish these cases: have it return an Optional Set
(or a small enum) where nil (or a MatchAll sentinel) means explicit
wildcard/class-or-id-only (preserve existing match-all behavior) but return an
actual empty Set for truly unrecognized tags so callers get zero matches; then
update query(...) to treat only the nil/MatchAll sentinel as matchAll and treat
an empty Set as matchNone. Ensure references: cssToAXRoles and query are
adjusted accordingly.
In `@libs/cua-driver/Sources/CuaDriverCore/Browser/BrowserJS.swift`:
- Around line 198-204: The current error detection uses fragile substring
matching on err to decide when to throw Error.javascriptNotEnabled(appName,
bundleId); change the check so it normalizes err (lowercase and trimmed) and
does a case-insensitive contains match (e.g., on "turned off" and "applescript
is turned off") to reduce locale/format fragility, and add a short TODO comment
near the p.terminationStatus / continuation.resume block noting the known
limitation and recommending future improvements (pre-checking target app bundle
permissions or a more robust AppleEvent/TCC check) if localization issues arise.
In `@libs/cua-driver/Sources/CuaDriverCore/Browser/CDPClient.swift`:
- Around line 83-87: Replace the force-unwrap when creating payloadStr: instead
of using String(data: try JSONSerialization.data(withJSONObject: payload),
encoding: .utf8)!, use a guard let on String(data: ..., encoding: .utf8) to
safely unwrap and, on failure, throw Error.connectionFailed("payload encoding");
update the code at the payloadStr creation site in CDPClient.swift (look for the
payloadStr variable and the JSONSerialization.data(withJSONObject:) call) so the
method's contract is explicit and avoids the force-unwrap smell.
- Around line 88-112: The evaluate path currently awaits a WebSocket
send/receive inside withCheckedThrowingContinuation with no deadline, so if the
inspector hangs the continuation is never resumed; modify the evaluate
implementation (the async method that creates the URLSession.webSocketTask,
calls ws.send and ws.receive and resumes the continuation) to impose a bounded
timeout by racing the continuation against a timeout Task (or using your
withTimeout helper), and if the timeout wins call continuation.resume(throwing:
Error.evaluationFailed("timeout after Ns")) and cancel the webSocketTask; ensure
all existing resume paths (ws send error, receive failure/success, and the new
timeout branch) still only resume once and that ws.cancel() is invoked when
timing out.
In `@libs/cua-driver/Sources/CuaDriverCore/Browser/ElectronJS.swift`:
- Around line 189-206: The runProcess helper (function runProcess) sets
proc.standardError = Pipe() but never reads it and only reads stdout lazily in
the terminationHandler, which can deadlock if stderr fills; fix by creating a
separate Pipe for stderr (e.g., let errPipe = Pipe()), read both
pipe.fileHandleForReading and errPipe.fileHandleForReading (collect
dataToEndOfFile) concurrently before/respecting termination, then combine or
discard stderr (or redirect to /dev/null) as callers expect, and ensure
continuation.resume is called exactly once with the aggregated stdout (and
optionally include stderr in the returned string or log it) while catching
proc.run() errors and cleaning up handlers (update references to proc, pipe,
standardError, terminationHandler, and runProcess).
In `@libs/cua-driver/Sources/CuaDriverCore/Browser/WebInspectorXPC.swift`:
- Around line 45-48: The current fragile heuristic using
bundlePath.contains("tauri") in WebInspectorXPC.swift incorrectly classifies
many apps as Tauri; remove this short-circuit and rely on the existing
WebKit-linkage probe (the otool -L check) for accurate detection, or if you must
keep a fast path, tighten it by resolving the app bundle's
Contents/MacOS/<executable> and checking that binary's linkage/symbols for known
Tauri/libwry identifiers before returning true (refer to bundleURL,
bundlePath.contains("tauri") and the WebKit-linkage probe logic to locate and
update the decision point).
- Around line 55-69: The isWKWebViewApp flow currently calls isLinkedToWebKit
(which runs runProcess("/usr/bin/otool", ...)) on every check, so add a static
cache (e.g., [URL: Bool] or [String: Bool] keyed by bundleURL or bundle
identifier) inside WebInspectorXPC to remember results per app; update
isWKWebViewApp to first consult this cache and return the cached Bool if
present, otherwise call isLinkedToWebKit, store the result in the cache, and
return it; ensure thread-safety around the cache (use a serial DispatchQueue or
lock) and keep references keyed by bundle/executable URL (or bundle id) to avoid
rerunning the expensive otool subprocess repeatedly.
In `@libs/cua-driver/Sources/CuaDriverCore/Browser/WebKitJS.swift`:
- Around line 9-11: Update the doc-comment to stop saying "near-instant" and
state the aggregate timeout (~1.5s for 3 ports at 0.5s each) and that probes can
be slow on macOS; then change the hot-path probe logic inside
WebKitJS.isAvailable() to avoid repeated full probes by caching a negative
result for the process lifetime (e.g. a static/cached Optional Bool used on
subsequent calls) and/or short-circuit early using platform or isWKWebViewApp
detection before performing the per-port timeouts so repeated page dispatches
don't incur the 1.5s cost.
In `@libs/cua-driver/Sources/CuaDriverCore/Permissions/Permissions.swift`:
- Around line 25-28: The doc comment for currentStatus() is stale—update it to
describe that the probe now uses the CGWindowList API with
excludingDesktopWindows(false, onScreenWindowsOnly: true) instead of
SCShareableContent.current; mention the rationale that this avoids the
multi-second hang caused by enumerating off-screen windows (per `#1371`) while
still accurately probing screen-recording permission, and adjust wording to note
the ~100–300ms probe cost if still applicable.
In `@libs/cua-driver/Tests/integration/test_browser_js.py`:
- Around line 283-284: The current fragile regex self.assertRegex(text,
r"elements.*turn", re.IGNORECASE) in test_browser_js.py is opaque and brittle;
replace it with an assertion that targets a stable AX tree marker emitted by
get_window_state (for example the fixed section header string that
get_window_state always includes, e.g., "AX Tree" or the exact header used in
get_window_state), and add a one-line comment above the assertion describing the
expected AX tree content and why that header is stable; update the assertion to
use self.assertIn(stable_header, text) or a precise regex matching that header
to make the test stable and readable.
In `@libs/cua-driver/Tests/integration/test_webkit_js.py`:
- Around line 178-183: Replace the brittle fixed sleep after calling
"launch_app" with a short polling loop that repeatedly calls _find_conductor_pid
until it returns a PID or a configurable timeout elapses; specifically, after
cls.client.call("launch_app", ...) remove time.sleep(3) and instead poll
_find_conductor_pid(cls.client) with a small interval (e.g. 0.1–0.5s) and a
total timeout (e.g. a few seconds) and set cls.pid when found, raising/asserting
a clear failure if the timeout elapses so the test fails fast on genuine launch
failures.
- Around line 137-148: The code incorrectly checks the parsed window_id with a
truthy test which treats 0 as missing; update the conditionals that check
window_id (both the outer check here and the inner loop in _get_window_id) to
use explicit "is not None" comparisons so 0 is treated as a valid CGWindowID and
the intent is clear; ensure you only call self.skipTest("No Chrome window
found") when window_id is None.
🪄 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: 3a6e4ec6-1a5b-4de7-b8ff-02510f949e3f
📒 Files selected for processing (12)
libs/cua-driver/Skills/cua-driver/SKILL.mdlibs/cua-driver/Sources/CuaDriverCore/Browser/AXPageReader.swiftlibs/cua-driver/Sources/CuaDriverCore/Browser/BrowserJS.swiftlibs/cua-driver/Sources/CuaDriverCore/Browser/CDPClient.swiftlibs/cua-driver/Sources/CuaDriverCore/Browser/ElectronJS.swiftlibs/cua-driver/Sources/CuaDriverCore/Browser/WebInspectorXPC.swiftlibs/cua-driver/Sources/CuaDriverCore/Browser/WebKitJS.swiftlibs/cua-driver/Sources/CuaDriverCore/Permissions/Permissions.swiftlibs/cua-driver/Sources/CuaDriverServer/Tools/PageTool.swiftlibs/cua-driver/Tests/integration/test_browser_js.pylibs/cua-driver/Tests/integration/test_webkit_js.pylibs/cua-driver/scripts/CuaDriver.entitlements
Comment on lines
+773
to
+775
| ``` | ||
| get_window_state({pid, window_id, javascript: "document.title"}) | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add a language to the fenced code block.
Static analysis (markdownlint MD040) flags the bare ``` opener at line 773. Use a language tag for syntax highlighting and lint compliance.
📝 Suggested fix
-```
+```text
get_window_state({pid, window_id, javascript: "document.title"})
</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 773-773: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 773 - 775, The
fenced code block containing get_window_state({pid, window_id, javascript:
"document.title"}) is missing a language tag which triggers markdownlint MD040;
update the triple-backtick opener to include a language (e.g., text or
javascript) so it becomes ```text (or ```javascript) and keep the block content
unchanged to provide proper syntax highlighting and lint compliance in SKILL.md.
Comment on lines
+221
to
+305
| public static func enableJavaScriptAppleEvents(bundleId: String) async throws { | ||
| guard let profilesDir = profilesDirectory(for: bundleId) else { | ||
| throw Error.unsupportedBrowser(bundleId) | ||
| } | ||
|
|
||
| // Terminate all running instances of the browser. | ||
| let running = await MainActor.run { | ||
| NSWorkspace.shared.runningApplications | ||
| .filter { $0.bundleIdentifier == bundleId } | ||
| } | ||
| for app in running { app.terminate() } | ||
|
|
||
| // Wait up to 5 s for the browser to quit (it writes Preferences on exit). | ||
| let deadline = Date().addingTimeInterval(5) | ||
| while Date() < deadline { | ||
| let still = await MainActor.run { | ||
| NSWorkspace.shared.runningApplications | ||
| .contains { $0.bundleIdentifier == bundleId } | ||
| } | ||
| if !still { break } | ||
| try await Task.sleep(nanoseconds: 200_000_000) | ||
| } | ||
|
|
||
| // Find all profile Preferences files and patch them. | ||
| let fm = FileManager.default | ||
| let profilesDirURL = URL(fileURLWithPath: profilesDir) | ||
| let profileDirs = (try? fm.contentsOfDirectory( | ||
| at: profilesDirURL, includingPropertiesForKeys: [.isDirectoryKey])) ?? [] | ||
| var patched = 0 | ||
| for dir in profileDirs { | ||
| let prefsURL = dir.appendingPathComponent("Preferences") | ||
| guard fm.fileExists(atPath: prefsURL.path), | ||
| let data = fm.contents(atPath: prefsURL.path), | ||
| var prefs = try? JSONSerialization.jsonObject(with: data) as? [String: Any] | ||
| else { continue } | ||
|
|
||
| var browser = prefs["browser"] as? [String: Any] ?? [:] | ||
| browser["allow_javascript_apple_events"] = true | ||
| prefs["browser"] = browser | ||
|
|
||
| // Chrome syncs this key under account_values.browser as well. | ||
| if bundleId == "com.google.Chrome" { | ||
| var acct = prefs["account_values"] as? [String: Any] ?? [:] | ||
| var acctBrowser = acct["browser"] as? [String: Any] ?? [:] | ||
| acctBrowser["allow_javascript_apple_events"] = true | ||
| acct["browser"] = acctBrowser | ||
| prefs["account_values"] = acct | ||
| } | ||
|
|
||
| guard let newData = try? JSONSerialization.data( | ||
| withJSONObject: prefs, options: [.prettyPrinted, .sortedKeys]) | ||
| else { continue } | ||
| try newData.write(to: prefsURL, options: .atomic) | ||
| patched += 1 | ||
| } | ||
|
|
||
| if patched == 0 { | ||
| throw Error.enableFailed( | ||
| "No Preferences files found under \(profilesDirURL.path). " | ||
| + "Ensure the browser has been launched at least once.") | ||
| } | ||
|
|
||
| // Relaunch the browser. | ||
| if let appURL = await MainActor.run(resultType: URL?.self, body: { | ||
| NSWorkspace.shared.urlForApplication(withBundleIdentifier: bundleId) | ||
| }) { | ||
| let cfg = NSWorkspace.OpenConfiguration() | ||
| try await NSWorkspace.shared.openApplication(at: appURL, configuration: cfg) | ||
| } | ||
| } | ||
|
|
||
| /// Returns the directory that contains per-profile subdirectories for this browser. | ||
| private static func profilesDirectory(for bundleId: String) -> String? { | ||
| let home = FileManager.default.homeDirectoryForCurrentUser.path | ||
| switch bundleId { | ||
| case "com.google.Chrome": | ||
| return "\(home)/Library/Application Support/Google/Chrome" | ||
| case "com.brave.Browser": | ||
| return "\(home)/Library/Application Support/BraveSoftware/Brave-Browser" | ||
| case "com.microsoft.edgemac": | ||
| return "\(home)/Library/Application Support/Microsoft Edge" | ||
| default: | ||
| return nil | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Safari is supports() == true but enableJavaScriptAppleEvents always throws for it.
supports("com.apple.Safari") returns true (via safariSpec()), but profilesDirectory(for: "com.apple.Safari") returns nil, so enableJavaScriptAppleEvents immediately throws .unsupportedBrowser("com.apple.Safari"). Safari's "Allow JavaScript from Apple Events" lives under Develop menu / com.apple.Safari defaults (AllowJavaScriptFromAppleEvents), not in a Preferences JSON file.
This is a discoverability hazard for callers — the consent message in javascriptNotEnabledMessage instructs the agent to call enable_javascript_apple_events with the bundle id, which will fail unhelpfully for Safari.
Either implement the Safari enable path (defaults write + Develop menu requirement) or have enableJavaScriptAppleEvents throw a clearer Safari-specific error explaining the manual steps. At minimum document the limitation in the consent message when bundleId == "com.apple.Safari".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/cua-driver/Sources/CuaDriverCore/Browser/BrowserJS.swift` around lines
221 - 305, The enableJavaScriptAppleEvents flow incorrectly treats Safari as
unsupported because profilesDirectory(for:) returns nil; update
enableJavaScriptAppleEvents to handle bundleId == "com.apple.Safari" specially
(or surface a clear Safari-specific error) by either: (A) implementing the
Safari path — run the appropriate defaults write for key
"AllowJavaScriptFromAppleEvents" and ensure the Develop menu requirement is
documented before returning, or (B) immediately throw a descriptive Error (e.g.
in enableJavaScriptAppleEvents when bundleId == "com.apple.Safari") that tells
callers Safari must be enabled via the Develop menu and the defaults write
command; also update javascriptNotEnabledMessage to include the Safari-specific
manual steps when bundleId == "com.apple.Safari". Use the symbols
enableJavaScriptAppleEvents(bundleId:), profilesDirectory(for:),
javascriptNotEnabledMessage and the Error enum to locate and implement the
change.
Comment on lines
+226
to
+242
| // Terminate all running instances of the browser. | ||
| let running = await MainActor.run { | ||
| NSWorkspace.shared.runningApplications | ||
| .filter { $0.bundleIdentifier == bundleId } | ||
| } | ||
| for app in running { app.terminate() } | ||
|
|
||
| // Wait up to 5 s for the browser to quit (it writes Preferences on exit). | ||
| let deadline = Date().addingTimeInterval(5) | ||
| while Date() < deadline { | ||
| let still = await MainActor.run { | ||
| NSWorkspace.shared.runningApplications | ||
| .contains { $0.bundleIdentifier == bundleId } | ||
| } | ||
| if !still { break } | ||
| try await Task.sleep(nanoseconds: 200_000_000) | ||
| } |
Contributor
There was a problem hiding this comment.
5 s deadline-poll for browser quit can race Preferences write.
If the browser doesn't terminate within 5 s (e.g., a "Save unsaved tabs?" dialog blocks shutdown), the loop exits with the browser still running, then JSONSerialization.data overwrites Preferences while Chrome may also write to it on actual exit — last-writer-wins on a JSON file the browser cares about. Worst case Chrome corrupts the file or wipes the patched flag.
Worth either failing fast (throw .enableFailed(...)) when the browser is still running after the deadline, or extending the wait and force-killing remaining instances after a polite terminate().
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/cua-driver/Sources/CuaDriverCore/Browser/BrowserJS.swift` around lines
226 - 242, The current 5s polling loop using MainActor.run over
NSWorkspace.shared.runningApplications (filtering by bundleId) can exit while
the browser is still running and then proceed to JSONSerialization.data, leading
to a race; change the logic so that after calling app.terminate() you either (a)
if any apps still exist after the deadline, throw an enable-failed error (e.g.,
throw .enableFailed(...)) to fail fast, or (b) extend the wait and for any
remaining apps call app.forceTerminate() and then wait until
NSWorkspace.shared.runningApplications no longer contains bundleId before
proceeding to write Preferences; ensure you reference the same bundleId, use
app.terminate()/app.forceTerminate(), and only continue to
JSONSerialization.data once no runningApplications contain the bundleId.
Comment on lines
+88
to
+111
| return try await withCheckedThrowingContinuation { continuation in | ||
| let ws = URLSession.shared.webSocketTask(with: wsURL) | ||
| ws.resume() | ||
| ws.send(.string(payloadStr)) { sendError in | ||
| if let err = sendError { | ||
| ws.cancel() | ||
| continuation.resume(throwing: Error.connectionFailed(err.localizedDescription)) | ||
| return | ||
| } | ||
| ws.receive { result in | ||
| ws.cancel() | ||
| switch result { | ||
| case .failure(let err): | ||
| continuation.resume(throwing: Error.connectionFailed(err.localizedDescription)) | ||
| case .success(let message): | ||
| guard case .string(let str) = message else { | ||
| continuation.resume(throwing: Error.connectionFailed("binary WebSocket frame")) | ||
| return | ||
| } | ||
| continuation.resume(with: Result { try Self.parseResult(str) }) | ||
| } | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Chrome DevTools Protocol guarantee that command response frames precede unsolicited event frames on a fresh WebSocket connection?
💡 Result:
No, the Chrome DevTools Protocol does not guarantee that command response frames precede unsolicited event frames on a fresh WebSocket connection.
Citations:
- 1: https://github.com/aslushnikov/getting-started-with-cdp/blob/HEAD/README.md
- 2: https://developers.chrome.com/docs/devtools/protocol-monitor
- 3: https://chromedevtools.github.io/devtools-protocol/
ws.receive reads exactly one frame without filtering by id — risks matching the wrong message.
Chrome DevTools Protocol does not guarantee that command response frames precede unsolicited event frames on a fresh WebSocket connection. The current code sends one message and reads exactly one frame, then feeds it to parseResult without verifying obj["id"] == 1.
If a notification arrives before the response—or on a target that auto-emits lifecycle events—parseResult will either fail with a confusing error or return wrong data. While the practical impact is low here (since Runtime.enable is not called and no event subscriptions are active), the receive loop should filter by id for correctness and resilience to future changes.
♻️ Suggested fix: loop until id matches
- ws.receive { result in
- ws.cancel()
- switch result {
- case .failure(let err):
- continuation.resume(throwing: Error.connectionFailed(err.localizedDescription))
- case .success(let message):
- guard case .string(let str) = message else {
- continuation.resume(throwing: Error.connectionFailed("binary WebSocket frame"))
- return
- }
- continuation.resume(with: Result { try Self.parseResult(str) })
- }
- }
+ Self.receiveMatching(ws: ws, id: 1, continuation: continuation)…with a helper that re-issues ws.receive while the incoming frame's id does not match, bounded by a timeout.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/cua-driver/Sources/CuaDriverCore/Browser/CDPClient.swift` around lines
88 - 111, The receive path may consume an unsolicited event frame because it
reads exactly one frame and passes it to parseResult; change CDPClient's
request/response flow to loop-reading ws.receive until a frame with a matching
"id" is found (or a bounded timeout occurs) instead of assuming the first frame
is the response. Implement a helper (e.g., receiveMatchingResponse(webSocket:
URLSessionWebSocketTask, expectedId: Int, timeout: TimeInterval)) that receives
frames, parses each string payload to JSON, inspects obj["id"] and returns the
string for the matching id (or throws Error.connectionFailed on timeout/parse
failure), then call parseResult only with that matched frame; ensure the
original continuation usage in withCheckedThrowingContinuation remains but uses
this helper instead of a single ws.receive.
Comment on lines
+96
to
+122
| // Poll up to 2 s for a new LISTEN port to appear. | ||
| var inspectorPort: Int? | ||
| for _ in 0..<10 { | ||
| try await Task.sleep(for: .milliseconds(200)) | ||
| let portsAfter = await listeningPorts(pid: pid) | ||
| let newPorts = portsAfter.subtracting(portsBefore) | ||
| for port in newPorts { | ||
| if await isInspectorPort(port) { | ||
| inspectorPort = port | ||
| break | ||
| } | ||
| } | ||
| if inspectorPort != nil { break } | ||
|
|
||
| // Fallback: scan the common Node inspector range in case lsof | ||
| // didn't resolve the pid association in time. | ||
| if let port = await scanInspectorPorts() { | ||
| inspectorPort = port | ||
| break | ||
| } | ||
| } | ||
|
|
||
| guard let port = inspectorPort else { | ||
| throw Error.inspectorNotAvailable( | ||
| "no CDP endpoint appeared on localhost within 2 s after SIGUSR1" | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
Polling-loop fallback scan can blow past the documented 2 s budget.
scanInspectorPorts() is invoked on every iteration of the 10-step poll loop, regardless of whether lsof produced new ports. Each call walks 9229...9249 sequentially via CDPClient.isAvailable (0.5 s timeout each) — worst case 21 × 0.5 s = ~10.5 s per iteration when nothing answers, multiplied by up to 10 iterations. That contradicts the doc-comment claim of "Poll up to 2 s" and the error message "no CDP endpoint appeared on localhost within 2 s after SIGUSR1".
Consider running the fallback scan once after the polling loop fails, not on every iteration. The lsof-diff path is the primary signal; the scan is only a defensive backstop.
♻️ Suggested restructure
- // Poll up to 2 s for a new LISTEN port to appear.
+ // Poll up to 2 s for a new LISTEN port to appear via lsof diff.
var inspectorPort: Int?
for _ in 0..<10 {
try await Task.sleep(for: .milliseconds(200))
let portsAfter = await listeningPorts(pid: pid)
let newPorts = portsAfter.subtracting(portsBefore)
for port in newPorts {
if await isInspectorPort(port) {
inspectorPort = port
break
}
}
if inspectorPort != nil { break }
-
- // Fallback: scan the common Node inspector range in case lsof
- // didn't resolve the pid association in time.
- if let port = await scanInspectorPorts() {
- inspectorPort = port
- break
- }
+ }
+
+ // Fallback: lsof may not have associated the new port to the pid in time.
+ if inspectorPort == nil {
+ inspectorPort = await scanInspectorPorts()
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Poll up to 2 s for a new LISTEN port to appear. | |
| var inspectorPort: Int? | |
| for _ in 0..<10 { | |
| try await Task.sleep(for: .milliseconds(200)) | |
| let portsAfter = await listeningPorts(pid: pid) | |
| let newPorts = portsAfter.subtracting(portsBefore) | |
| for port in newPorts { | |
| if await isInspectorPort(port) { | |
| inspectorPort = port | |
| break | |
| } | |
| } | |
| if inspectorPort != nil { break } | |
| // Fallback: scan the common Node inspector range in case lsof | |
| // didn't resolve the pid association in time. | |
| if let port = await scanInspectorPorts() { | |
| inspectorPort = port | |
| break | |
| } | |
| } | |
| guard let port = inspectorPort else { | |
| throw Error.inspectorNotAvailable( | |
| "no CDP endpoint appeared on localhost within 2 s after SIGUSR1" | |
| ) | |
| } | |
| // Poll up to 2 s for a new LISTEN port to appear via lsof diff. | |
| var inspectorPort: Int? | |
| for _ in 0..<10 { | |
| try await Task.sleep(for: .milliseconds(200)) | |
| let portsAfter = await listeningPorts(pid: pid) | |
| let newPorts = portsAfter.subtracting(portsBefore) | |
| for port in newPorts { | |
| if await isInspectorPort(port) { | |
| inspectorPort = port | |
| break | |
| } | |
| } | |
| if inspectorPort != nil { break } | |
| } | |
| // Fallback: lsof may not have associated the new port to the pid in time. | |
| if inspectorPort == nil { | |
| inspectorPort = await scanInspectorPorts() | |
| } | |
| guard let port = inspectorPort else { | |
| throw Error.inspectorNotAvailable( | |
| "no CDP endpoint appeared on localhost within 2 s after SIGUSR1" | |
| ) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/cua-driver/Sources/CuaDriverCore/Browser/ElectronJS.swift` around lines
96 - 122, The polling loop currently calls scanInspectorPorts() on every
iteration which can massively exceed the documented 2s budget; change the flow
so the loop only polls listeningPorts(pid:) and checks isInspectorPort(_:) for
newPorts for up to 10 iterations, and if inspectorPort is still nil after the
loop run scanInspectorPorts() exactly once as a final fallback (using its result
to set inspectorPort before the guard/throw). Update references: remove the
scanInspectorPorts() call from inside the for _ in 0..<10 loop and add a single
call to scanInspectorPorts() after the loop that assigns inspectorPort if
non-nil.
Comment on lines
+165
to
+231
| case "get_text": | ||
| // For WKWebView/Tauri apps, skip JS injection and read the AX tree | ||
| // directly — avoids the entitlement-blocked inspector timeout. | ||
| if WebInspectorXPC.isWKWebViewApp(pid: pid) { | ||
| if let axText = await axGetText(pid: pid, windowId: windowId) { | ||
| return okResult("## Page text (via AX tree)\n\n\(axText)") | ||
| } | ||
| } | ||
| do { | ||
| let result = try await executeJS( | ||
| "document.body.innerText", | ||
| bundleId: bundleId, pid: pid, windowId: windowId) | ||
| return okResult(result) | ||
| } catch { | ||
| if let axText = await axGetText(pid: pid, windowId: windowId) { | ||
| return okResult("## Page text (via AX tree)\n\n\(axText)") | ||
| } | ||
| return errorResult("\(error)") | ||
| } | ||
|
|
||
| case "query_dom": | ||
| guard let selector = arguments?["css_selector"]?.stringValue, | ||
| !selector.isEmpty | ||
| else { | ||
| return errorResult( | ||
| "action=query_dom requires a non-empty css_selector field.") | ||
| } | ||
| // For WKWebView/Tauri apps, go straight to AX role query. | ||
| if WebInspectorXPC.isWKWebViewApp(pid: pid) { | ||
| if let axResult = await axQueryDom( | ||
| selector: selector, pid: pid, windowId: windowId) { | ||
| return okResult( | ||
| "## AX query: `\(selector)` (via accessibility tree)\n\n```json\n\(axResult)\n```") | ||
| } | ||
| } | ||
| let attrs: [String] | ||
| if let rawAttrs = arguments?["attributes"]?.arrayValue { | ||
| attrs = rawAttrs.compactMap { $0.stringValue } | ||
| } else { | ||
| attrs = [] | ||
| } | ||
| let attrJS = attrs.isEmpty | ||
| ? "[]" | ||
| : "[\(attrs.map { "\"\($0)\"" }.joined(separator: ", "))]" | ||
| let js = """ | ||
| (() => { | ||
| const attrs = \(attrJS); | ||
| return JSON.stringify( | ||
| Array.from(document.querySelectorAll(\(jsonString(selector)))).map(el => { | ||
| const obj = { tag: el.tagName.toLowerCase(), text: el.innerText?.trim() }; | ||
| for (const a of attrs) obj[a] = el.getAttribute(a); | ||
| return obj; | ||
| }) | ||
| ); | ||
| })() | ||
| """ | ||
| do { | ||
| let result = try await executeJS(js, bundleId: bundleId, pid: pid, windowId: windowId) | ||
| return okResult("## DOM query: `\(selector)`\n\n```json\n\(result)\n```") | ||
| } catch { | ||
| if let axResult = await axQueryDom( | ||
| selector: selector, pid: pid, windowId: windowId) { | ||
| return okResult( | ||
| "## AX query: `\(selector)` (via accessibility tree)\n\n```json\n\(axResult)\n```") | ||
| } | ||
| return errorResult("\(error)") | ||
| } |
Contributor
There was a problem hiding this comment.
WKWebView fall-through emits a misleading "execute_javascript not available" error for get_text/query_dom.
For a WKWebView/Tauri PID, when the AX helper returns nil (empty page extraction, or — for query_dom — zero matches because axQueryDom returns nil instead of an empty array), control falls through to executeJS which throws WKWebViewJSUnavailableError. The user invoked get_text / query_dom, but the surfaced error talks about execute_javascript and points them away from the very tool they just used.
Most impactful for query_dom: a selector with zero matches is a normal outcome that should produce an empty JSON array, not an error.
Two issues to fix:
- Don't fall through to JS for known-WKWebView apps — once the AX path is chosen it's the only viable backend.
- Disambiguate
axQueryDom"snapshot failed" from "no matches" so the latter returns an empty array.
🛡️ Proposed fix
case "get_text":
if WebInspectorXPC.isWKWebViewApp(pid: pid) {
- if let axText = await axGetText(pid: pid, windowId: windowId) {
- return okResult("## Page text (via AX tree)\n\n\(axText)")
- }
+ let axText = await axGetText(pid: pid, windowId: windowId) ?? ""
+ return okResult("## Page text (via AX tree)\n\n\(axText)")
}
...
case "query_dom":
...
if WebInspectorXPC.isWKWebViewApp(pid: pid) {
- if let axResult = await axQueryDom(
- selector: selector, pid: pid, windowId: windowId) {
- return okResult(
- "## AX query: `\(selector)` (via accessibility tree)\n\n```json\n\(axResult)\n```")
- }
+ let axResult = await axQueryDom(
+ selector: selector, pid: pid, windowId: windowId) ?? "[]"
+ return okResult(
+ "## AX query: `\(selector)` (via accessibility tree)\n\n```json\n\(axResult)\n```")
}…and have axQueryDom return "[]" (or distinguish nil snapshot vs. no matches) instead of nil when the snapshot succeeded but no elements matched.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| case "get_text": | |
| // For WKWebView/Tauri apps, skip JS injection and read the AX tree | |
| // directly — avoids the entitlement-blocked inspector timeout. | |
| if WebInspectorXPC.isWKWebViewApp(pid: pid) { | |
| if let axText = await axGetText(pid: pid, windowId: windowId) { | |
| return okResult("## Page text (via AX tree)\n\n\(axText)") | |
| } | |
| } | |
| do { | |
| let result = try await executeJS( | |
| "document.body.innerText", | |
| bundleId: bundleId, pid: pid, windowId: windowId) | |
| return okResult(result) | |
| } catch { | |
| if let axText = await axGetText(pid: pid, windowId: windowId) { | |
| return okResult("## Page text (via AX tree)\n\n\(axText)") | |
| } | |
| return errorResult("\(error)") | |
| } | |
| case "query_dom": | |
| guard let selector = arguments?["css_selector"]?.stringValue, | |
| !selector.isEmpty | |
| else { | |
| return errorResult( | |
| "action=query_dom requires a non-empty css_selector field.") | |
| } | |
| // For WKWebView/Tauri apps, go straight to AX role query. | |
| if WebInspectorXPC.isWKWebViewApp(pid: pid) { | |
| if let axResult = await axQueryDom( | |
| selector: selector, pid: pid, windowId: windowId) { | |
| return okResult( | |
| "## AX query: `\(selector)` (via accessibility tree)\n\n```json\n\(axResult)\n```") | |
| } | |
| } | |
| let attrs: [String] | |
| if let rawAttrs = arguments?["attributes"]?.arrayValue { | |
| attrs = rawAttrs.compactMap { $0.stringValue } | |
| } else { | |
| attrs = [] | |
| } | |
| let attrJS = attrs.isEmpty | |
| ? "[]" | |
| : "[\(attrs.map { "\"\($0)\"" }.joined(separator: ", "))]" | |
| let js = """ | |
| (() => { | |
| const attrs = \(attrJS); | |
| return JSON.stringify( | |
| Array.from(document.querySelectorAll(\(jsonString(selector)))).map(el => { | |
| const obj = { tag: el.tagName.toLowerCase(), text: el.innerText?.trim() }; | |
| for (const a of attrs) obj[a] = el.getAttribute(a); | |
| return obj; | |
| }) | |
| ); | |
| })() | |
| """ | |
| do { | |
| let result = try await executeJS(js, bundleId: bundleId, pid: pid, windowId: windowId) | |
| return okResult("## DOM query: `\(selector)`\n\n```json\n\(result)\n```") | |
| } catch { | |
| if let axResult = await axQueryDom( | |
| selector: selector, pid: pid, windowId: windowId) { | |
| return okResult( | |
| "## AX query: `\(selector)` (via accessibility tree)\n\n```json\n\(axResult)\n```") | |
| } | |
| return errorResult("\(error)") | |
| } | |
| case "get_text": | |
| // For WKWebView/Tauri apps, skip JS injection and read the AX tree | |
| // directly — avoids the entitlement-blocked inspector timeout. | |
| if WebInspectorXPC.isWKWebViewApp(pid: pid) { | |
| let axText = await axGetText(pid: pid, windowId: windowId) ?? "" | |
| return okResult("## Page text (via AX tree)\n\n\(axText)") | |
| } | |
| do { | |
| let result = try await executeJS( | |
| "document.body.innerText", | |
| bundleId: bundleId, pid: pid, windowId: windowId) | |
| return okResult(result) | |
| } catch { | |
| if let axText = await axGetText(pid: pid, windowId: windowId) { | |
| return okResult("## Page text (via AX tree)\n\n\(axText)") | |
| } | |
| return errorResult("\(error)") | |
| } | |
| case "query_dom": | |
| guard let selector = arguments?["css_selector"]?.stringValue, | |
| !selector.isEmpty | |
| else { | |
| return errorResult( | |
| "action=query_dom requires a non-empty css_selector field.") | |
| } | |
| // For WKWebView/Tauri apps, go straight to AX role query. | |
| if WebInspectorXPC.isWKWebViewApp(pid: pid) { | |
| let axResult = await axQueryDom( | |
| selector: selector, pid: pid, windowId: windowId) ?? "[]" | |
| return okResult( | |
| "## AX query: `\(selector)` (via accessibility tree)\n\n |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/cua-driver/Sources/CuaDriverServer/Tools/PageTool.swift` around lines
165 - 231, For the WKWebView/Tauri code paths in the get_text and query_dom
cases, stop falling through to executeJS when
WebInspectorXPC.isWKWebViewApp(pid:) is true; if axGetText(...) or
axQueryDom(...) returns nil because the snapshot succeeded but had no matches,
treat that as a valid empty result (return okResult with "## Page text (via AX
tree)\n\n\(axText)" for get_text and for query_dom return okResult with an empty
JSON array e.g. "[]"); update the query_dom branch to coalesce axQueryDom(...)
to "[]" (or change axQueryDom to return "[]" when snapshot succeeded with zero
matches and only return nil for true snapshot failures) and ensure executeJS is
only attempted when not a WKWebView app.
Comment on lines
+165
to
+199
| case "get_text": | ||
| // For WKWebView/Tauri apps, skip JS injection and read the AX tree | ||
| // directly — avoids the entitlement-blocked inspector timeout. | ||
| if WebInspectorXPC.isWKWebViewApp(pid: pid) { | ||
| if let axText = await axGetText(pid: pid, windowId: windowId) { | ||
| return okResult("## Page text (via AX tree)\n\n\(axText)") | ||
| } | ||
| } | ||
| do { | ||
| let result = try await executeJS( | ||
| "document.body.innerText", | ||
| bundleId: bundleId, pid: pid, windowId: windowId) | ||
| return okResult(result) | ||
| } catch { | ||
| if let axText = await axGetText(pid: pid, windowId: windowId) { | ||
| return okResult("## Page text (via AX tree)\n\n\(axText)") | ||
| } | ||
| return errorResult("\(error)") | ||
| } | ||
|
|
||
| case "query_dom": | ||
| guard let selector = arguments?["css_selector"]?.stringValue, | ||
| !selector.isEmpty | ||
| else { | ||
| return errorResult( | ||
| "action=query_dom requires a non-empty css_selector field.") | ||
| } | ||
| // For WKWebView/Tauri apps, go straight to AX role query. | ||
| if WebInspectorXPC.isWKWebViewApp(pid: pid) { | ||
| if let axResult = await axQueryDom( | ||
| selector: selector, pid: pid, windowId: windowId) { | ||
| return okResult( | ||
| "## AX query: `\(selector)` (via accessibility tree)\n\n```json\n\(axResult)\n```") | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate isLinkedToWebKit implementation and the full isWKWebViewApp flow.
fd -t f WebInspectorXPC.swift
ast-grep --pattern 'func isLinkedToWebKit($$$) { $$$ }'
ast-grep --pattern 'public static func isWKWebViewApp($$$) -> Bool { $$$ }'
# And confirm BrowserJS.supports list to identify candidates that should bypass the WKWebView gate.
ast-grep --pattern 'public static func supports(bundleId: String) -> Bool { $$$ }'Repository: trycua/cua
Length of output: 3839
🏁 Script executed:
# Read the full WebInspectorXPC.swift file to see isLinkedToWebKit implementation
cat -n libs/cua-driver/Sources/CuaDriverCore/Browser/WebInspectorXPC.swift | head -100Repository: trycua/cua
Length of output: 4315
🏁 Script executed:
# Check BrowserJS.swift for Safari handling and browserSpec
rg -A 20 "func browserSpec" libs/cua-driver/Sources/CuaDriverCore/Browser/BrowserJS.swift
rg "safari\|Safari" libs/cua-driver/Sources/CuaDriverCore/Browser/BrowserJS.swift -iRepository: trycua/cua
Length of output: 948
🏁 Script executed:
# Check the execute_javascript case in PageTool.swift to see how it handles Safari
rg -B 5 -A 20 'case "execute_javascript"' libs/cua-driver/Sources/CuaDriverServer/Tools/PageTool.swiftRepository: trycua/cua
Length of output: 1307
🏁 Script executed:
# Also check if there's any Safari-specific logic elsewhere in PageTool
rg -i "safari\|com\.apple\.safari" libs/cua-driver/Sources/CuaDriverServer/Tools/PageTool.swiftRepository: trycua/cua
Length of output: 36
🏁 Script executed:
# Check the executeJS function to see if it validates against BrowserJS.supports
rg -B 5 -A 30 "func executeJS" libs/cua-driver/Sources/CuaDriverServer/Tools/PageTool.swiftRepository: trycua/cua
Length of output: 2040
🏁 Script executed:
# Also verify Safari's bundle ID is indeed com.apple.Safari
rg "com\.apple\.Safari" libs/cua-driver/Sources/CuaDriverCore/Browser/Repository: trycua/cua
Length of output: 292
🏁 Script executed:
# Check if there's any comment or code that explicitly addresses Safari in get_text or query_dom
rg -B 5 -A 5 "get_text\|query_dom" libs/cua-driver/Sources/CuaDriverServer/Tools/PageTool.swift | grep -A 5 -B 5 -i "safari\|browserjs\|supports"Repository: trycua/cua
Length of output: 36
🏁 Script executed:
# Verify Safari bundle ID matches what BrowserJS expects
rg "com\.apple\.Safari" libs/cua-driver/Sources/ -rRepository: trycua/cua
Length of output: 468
🏁 Script executed:
# Search for Safari references in the codebase
rg "Safari" libs/cua-driver/Sources/CuaDriverCore/Browser/Repository: trycua/cua
Length of output: 817
🏁 Script executed:
# Let me check what Safari's executable would report with otool
# Since we can't actually run otool, let's verify the logic more carefully.
# Check if there are any comments or documentation about Safari handling
rg -i "safari" libs/cua-driver/Sources/CuaDriverServer/Tools/PageTool.swiftRepository: trycua/cua
Length of output: 358
Safari will be incorrectly routed to the AX-tree fallback for get_text and query_dom.
Safari's executable is dynamically linked to WebKit (a system framework on macOS), so WebInspectorXPC.isWKWebViewApp(pid:) returns true at lines 168 and 193. Unlike execute_javascript which checks BrowserJS.supports(bundleId:) first, the get_text and query_dom cases evaluate the WKWebView gate before JS injection, causing Safari users to receive:
get_text: AX-tree extraction instead ofdocument.body.innerText(different content and markdown header).query_dom:{ role, text, element_index }rows instead of{ tag, text, …attrs }— breaking the documented schema that clients rely on.
Add a BrowserJS.supports(bundleId:) check ahead of the WKWebView gate (or exempt Safari from isWKWebViewApp) so Safari uses the JS path consistently across all three actions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/cua-driver/Sources/CuaDriverServer/Tools/PageTool.swift` around lines
165 - 199, The get_text and query_dom branches incorrectly route Safari to the
AX-tree fallback because they test WebInspectorXPC.isWKWebViewApp(pid:) before
checking JavaScript support; update both cases to first call
BrowserJS.supports(bundleId:) (same guard used by executeJS) and, only if JS is
unsupported, then fall back to WebInspectorXPC.isWKWebViewApp(pid:) and the
axGetText/axQueryDom paths; keep executeJS/axGetText/axQueryDom calls and error
handling unchanged so WKWebView/Tauri apps without JS support still use the
AX-tree fallback.
Comment on lines
+200
to
+220
| let attrs: [String] | ||
| if let rawAttrs = arguments?["attributes"]?.arrayValue { | ||
| attrs = rawAttrs.compactMap { $0.stringValue } | ||
| } else { | ||
| attrs = [] | ||
| } | ||
| let attrJS = attrs.isEmpty | ||
| ? "[]" | ||
| : "[\(attrs.map { "\"\($0)\"" }.joined(separator: ", "))]" | ||
| let js = """ | ||
| (() => { | ||
| const attrs = \(attrJS); | ||
| return JSON.stringify( | ||
| Array.from(document.querySelectorAll(\(jsonString(selector)))).map(el => { | ||
| const obj = { tag: el.tagName.toLowerCase(), text: el.innerText?.trim() }; | ||
| for (const a of attrs) obj[a] = el.getAttribute(a); | ||
| return obj; | ||
| }) | ||
| ); | ||
| })() | ||
| """ |
Contributor
There was a problem hiding this comment.
Attribute names are interpolated into JS without escaping.
selector is correctly escaped via jsonString on line 213, but attribute names on line 208 are wrapped in literal quotes only:
"[\(attrs.map { "\"\($0)\"" }.joined(separator: ", "))]"Any attribute name containing ", \, or a newline breaks the generated JS (and, since attributes is caller-supplied via the MCP arguments, an LLM-driven prompt-injection could craft strings that change the meaning of the snippet). Run each attribute name through the same jsonString helper used for the selector.
🛡️ Proposed fix
- let attrJS = attrs.isEmpty
- ? "[]"
- : "[\(attrs.map { "\"\($0)\"" }.joined(separator: ", "))]"
+ let attrJS = "[\(attrs.map { jsonString($0) }.joined(separator: ", "))]"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/cua-driver/Sources/CuaDriverServer/Tools/PageTool.swift` around lines
200 - 220, Attribute names in attrs are interpolated directly into attrJS
without escaping, which can break the generated JS or allow injection; update
the attrJS construction to run each attribute name through the existing
jsonString helper (the same way selector is escaped) before joining them so that
attrJS uses the safely-escaped values. Specifically, transform the attrs ->
attrJS mapping to use jsonString(...) for each element (referencing attrs,
attrJS and jsonString in PageTool.swift) so attribute names containing quotes,
backslashes, or newlines are properly escaped.
Comment on lines
+69
to
+79
| try: | ||
| with open(prefs_path) as f: | ||
| data = json.load(f) | ||
| data.setdefault("browser", {})["allow_javascript_apple_events"] = True | ||
| data.setdefault("account_values", {}).setdefault("browser", {})[ | ||
| "allow_javascript_apple_events" | ||
| ] = True | ||
| with open(prefs_path, "w") as f: | ||
| json.dump(data, f) | ||
| except Exception as e: | ||
| print(f" skipped {profile}: {e}") |
Contributor
There was a problem hiding this comment.
Non-atomic Preferences write can corrupt Chrome state.
Opening prefs_path with mode "w" truncates the file before json.dump writes anything. If the test is interrupted (Ctrl-C, OOM, crash) between the open and a successful dump, the user's Chrome Preferences file is left empty or partial and the profile may fail to start.
Write to a temp file in the same directory and atomically rename:
🛡️ Suggested fix
- with open(prefs_path, "w") as f:
- json.dump(data, f)
+ tmp = prefs_path + ".tmp"
+ with open(tmp, "w", encoding="utf-8") as f:
+ json.dump(data, f, ensure_ascii=False)
+ os.replace(tmp, prefs_path)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/cua-driver/Tests/integration/test_browser_js.py` around lines 69 - 79,
The prefs write is non-atomic and can corrupt Chrome if interrupted; update the
block that opens prefs_path to write to a temp file in the same directory (use
tempfile.NamedTemporaryFile or create prefs_path + ".tmp" in
os.path.dirname(prefs_path)), json.dump the data to that temp file, fsync and
close it, then atomically replace the original with os.replace(temp_path,
prefs_path); ensure you only replace after successful dump and consider
preserving permissions if needed.
Comment on lines
+242
to
+277
| @unittest.skipUnless(_conductor_installed(), "Conductor not installed") | ||
| def test_query_dom_buttons_via_ax_tree(self): | ||
| """query_dom(button) on Conductor returns AX buttons.""" | ||
| window_id = self._get_window_id() | ||
| if window_id is None: | ||
| self.skipTest("Conductor window not found") | ||
| result = self.client.call("page", { | ||
| "pid": self.pid, | ||
| "window_id": window_id, | ||
| "action": "query_dom", | ||
| "css_selector": "button", | ||
| }) | ||
| text = _tool_text(result) | ||
| # Either returns buttons or returns a well-formed error — not a crash. | ||
| self.assertFalse( | ||
| "Traceback" in text or "fatal" in text.lower(), | ||
| f"Unexpected crash output: {text}" | ||
| ) | ||
|
|
||
| @unittest.skipUnless(_conductor_installed(), "Conductor not installed") | ||
| def test_query_dom_links_via_ax_tree(self): | ||
| """query_dom(a) on Conductor returns AX links.""" | ||
| window_id = self._get_window_id() | ||
| if window_id is None: | ||
| self.skipTest("Conductor window not found") | ||
| result = self.client.call("page", { | ||
| "pid": self.pid, | ||
| "window_id": window_id, | ||
| "action": "query_dom", | ||
| "css_selector": "a", | ||
| }) | ||
| text = _tool_text(result) | ||
| self.assertFalse( | ||
| "Traceback" in text or "fatal" in text.lower(), | ||
| f"Unexpected crash output: {text}" | ||
| ) |
Contributor
There was a problem hiding this comment.
query_dom tests don't actually verify AX-fallback returned any data.
Both test_query_dom_buttons_via_ax_tree and test_query_dom_links_via_ax_tree only assert that the output does not contain "Traceback" or "fatal" — they pass if the tool returns an empty result, an error message, or anything else non-crashy. Per the docstring ("returns AX buttons") the intent looks stronger than the actual coverage.
If the AX fallback regresses to silently returning nothing, these tests still pass. Consider asserting _is_error(result) is False plus a non-empty payload, with a skipTest escape if Conductor genuinely renders no buttons/links on first launch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/cua-driver/Tests/integration/test_webkit_js.py` around lines 242 - 277,
The tests test_query_dom_buttons_via_ax_tree and
test_query_dom_links_via_ax_tree only assert the output is not a crash but don't
verify the AX-fallback actually returned data; change each test to assert the
call result is not an error (use _is_error(result) is False) and that the
payload contains at least one element (non-empty list/field returned by the
query_dom response), and if the page legitimately has no matching elements on
first launch then call self.skipTest with an explanatory message; locate the
checks inside those two test methods (they already build result via
self.client.call("page", {...}) and compute text = _tool_text(result)) and add
the _is_error and non-empty payload assertions immediately after obtaining
result/text, keeping the existing crash assertions as secondary safeguards.
- AXPageReader: extract page text and query DOM from WKWebView AX tree, used as JS-free backend for Tauri apps where the inspector is blocked - WebInspectorXPC: detect WKWebView apps; stub Mach IPC client for future use once the private entitlement becomes available - CDPClient: shared CDP HTTP+WebSocket eval extracted from ElectronJS - WebKitJS: probe GTK/WPE WebKit TCP inspector ports (Linux/fallback path) - ElectronJS: refactored to delegate to CDPClient - PageTool: route get_text/query_dom through AX tree for WKWebView apps; execute_javascript returns a clear error with AX-based alternatives - LaunchAppTool: add webkit_inspector_port and electron_debugging_port params - Permissions: fix SCShareableContent.current → excludingDesktopWindows (avoids 6s hang on machines with many ghost windows, fixes #1371) - test_webkit_js: new integration tests for WKWebView/Tauri AX path - test_browser_js: fix timing (4s wait for page load) and fix isWKWebViewApp false-positive for Chrome Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- AXPageReader.extractText: add AXWebArea to text-bearing roles; use description as tertiary fallback (title → value → description) since WebKit/Tauri AX trees often store text content in AXDescription. - PageTool.get_text / query_dom: check BrowserJS.supports() before routing to AX fallback so Safari (which links WebKit.framework) is NOT misidentified as a WKWebView app and incorrectly given AX output instead of document.body.innerText / querySelectorAll results. - PageTool.axGetText: fall back to raw treeMarkdown when extractText finds nothing, so WKWebView apps with limited AX exposure still return useful content instead of dropping through to executeJS (which always fails for WKWebView apps). - PageTool.query_dom: use jsonString() to escape attribute names before injecting them into the generated JS array literal, preventing XSS via malformed attribute values like `data-foo"; alert(1);//`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ebView features - mcp-tools.mdx: add 'page' tool section (Browser category) documenting execute_javascript, get_text, query_dom, and enable_javascript_apple_events actions including the AX fallback for WKWebView/Tauri apps. - mcp-tools.mdx: update launch_app docs to include creates_new_application_instance and additional_arguments parameters added in PR 1388. - installation.mdx: mark the LaunchAgent uninstall step as legacy (≤ v0.0.5) since the auto-updater LaunchAgent was removed in PR 1388. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
A single ws.receive assumed the next WebSocket frame was the response to the request. Chrome DevTools Protocol sends unsolicited event frames (Runtime.executionContextCreated, etc.) that arrive before responses, causing hangs or wrong-frame parses. Replace with a receive loop that discards frames with a "method" key (CDP events) and only resumes when a frame with a matching "id" arrives. Also adds a 10-second timeout to prevent indefinite hangs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
f-trycua
force-pushed
the
feat/cua-driver-wkwebview-tauri-ax
branch
from
April 26, 2026 07:48
7899e32 to
f01e89f
Compare
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
com.apple.private.webinspector.remote-inspection-debuggerprivate entitlementget_textandquery_domroute through AX tree for WKWebView/Tauri apps;execute_javascriptreturns a clear error pointing to the AX alternativeswebkit_inspector_portlaunches Tauri withTAURI_WEBVIEW_AUTOMATION=1;electron_debugging_portlaunches Electron with--remote-debugging-port=NSCShareableContent.current→excludingDesktopWindows(false, onScreenWindowsOnly: true)— avoids 6s hang on machines with many off-screen windows (fixes Performance regression with using SCShareableContent.current #1371)isWKWebViewAppfalse-positive for Chrome/Brave/EdgeTest plan
scripts/test.sh test_webkit_js— all non-inspector tests pass (10 pass, 8 skip)scripts/test.sh test_browser_js— all 11 passCloses #1371
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Documentation