diff --git a/native/computer-use-macos/Sources/OrcaComputerUseMacOS/main.swift b/native/computer-use-macos/Sources/OrcaComputerUseMacOS/main.swift index e0c3609fa52..5f341d0c16a 100644 --- a/native/computer-use-macos/Sources/OrcaComputerUseMacOS/main.swift +++ b/native/computer-use-macos/Sources/OrcaComputerUseMacOS/main.swift @@ -159,6 +159,12 @@ struct Snapshot { let elements: [Int: ElementRecord] let truncated: Bool let maxDepthReached: Bool + /// On-screen windows of the app (menus and sheets included), so a new or closed window counts as a change. + let appWindowIds: [CGWindowID] + + func showsSameState(as other: Snapshot) -> Bool { + treeText == other.treeText && windowBounds == other.windowBounds && appWindowIds == other.appWindowIds + } func withoutScreenshotPayload() -> Snapshot { Snapshot( @@ -176,7 +182,8 @@ struct Snapshot { screenshotEngine: nil, elements: elements, truncated: truncated, - maxDepthReached: maxDepthReached + maxDepthReached: maxDepthReached, + appWindowIds: appWindowIds ) } } @@ -209,6 +216,11 @@ final class Provider { private var snapshots: [String: Snapshot] = [:] private var snapshotEntries: [CachedSnapshotEntry] = [] + init() { + // Why: the system-wide element sets the wait for every AX call in this process, so a hung app costs one bounded timeout whatever the macOS default. + _ = AXUIElementSetMessagingTimeout(AXUIElementCreateSystemWide(), AccessibilityMessaging.timeoutSeconds) + } + func handle(method: String, params: [String: JSONValue]) throws -> Any { switch method { case "handshake": @@ -246,8 +258,21 @@ final class Provider { var action = try runAction() // Why: only --restore-window activates the app or posts to the HID tap; every other route leaves the user's desktop alone. action["delivery"] = isForegroundRequest(params) ? "foreground" : "background" + // Why: the action's currentSnapshot just cached this target's tree from before the action. + let before = action["path"] as? String == "accessibility" && action["verification"] == nil + ? try? cachedSnapshot(params: params) + : nil do { - return try renderActionResult(action: action, snapshot: observe(params: params)) + let after = try observe(params: params) + guard let before, after.showsSameState(as: before) else { + return renderActionResult(action: action, snapshot: after) + } + if let changed = try observeLateChange(before: before, params: params) { + return renderActionResult(action: action, snapshot: changed) + } + // Why: an AX call can succeed and do nothing, as AXPress does on many web controls (#31758). + action["verification"] = unverifiedAction(reason: "suspected_noop") + return renderActionResult(action: action, snapshot: after) } catch let error as ProviderError where (error.code == "window_not_found" || error.code == "window_stale") && hasRequestedWindowSelector(params) { var fallbackParams = params fallbackParams.removeValue(forKey: "windowId") @@ -259,6 +284,23 @@ final class Provider { } } + /// Re-reads the target for about a second, since apps often apply an action on a later run-loop turn. + /// Returns the observation to report once something changed, or nil if nothing did. + private func observeLateChange(before: Snapshot, params: [String: JSONValue]) throws -> Snapshot? { + let settleParams = params.merging(["restoreWindow": .bool(false)]) { _, replacement in replacement } + let quietParams = settleParams.merging(["noScreenshot": .bool(true)]) { _, replacement in replacement } + let deadline = Date().addingTimeInterval(1) + while Date() < deadline { + Thread.sleep(forTimeInterval: 0.2) + let polled = try observe(params: quietParams) + if !polled.showsSameState(as: before) { + if params["noScreenshot"]?.bool == true { return polled } + return try observe(params: settleParams) + } + } + return nil + } + private func observe(params: [String: JSONValue]) throws -> Snapshot { let query = try requiredString(params, "app") let windowId = try requestedWindowId(params) @@ -614,6 +656,12 @@ final class Provider { ) } let appElement = AXUIElementCreateApplication(app.pid) + var role: CFTypeRef? + let probeStart = CFAbsoluteTimeGetCurrent() + // Why: a hung app costs one timeout per AX read; walking it anyway held the request lock for every agent until the 60 s RPC timeout. + if AccessibilityMessaging.timedOut(AXUIElementCopyAttributeValue(appElement, kAXRoleAttribute as CFString, &role), since: probeStart) { + throw AccessibilityMessaging.appNotResponding(app.name) + } enableManualAccessibilityIfNeeded(appElement, app: app) let windowCandidates = WindowCapture.candidates(pid: app.pid) let focused = try focusedWindow( @@ -643,6 +691,9 @@ final class Provider { compactBrowserTabs: app.isKnownBrowser ) renderer.render(window) + if renderer.appStoppedResponding { + throw AccessibilityMessaging.appNotResponding(app.name) + } let screenshot = includeScreenshot ? capture.screenshotPayload() : nil let screenshotStatus: ScreenshotStatus = if screenshot != nil { .captured @@ -668,7 +719,8 @@ final class Provider { screenshotEngine: capture.image?.engine, elements: renderer.records, truncated: renderer.truncated, - maxDepthReached: renderer.maxDepthReached + maxDepthReached: renderer.maxDepthReached, + appWindowIds: windowCandidates.map(\.windowId).sorted() ) } @@ -775,10 +827,10 @@ final class Provider { private func performClickAction(record: ElementRecord, mouseButton: MouseButtonSelection) throws -> String? { if mouseButton == .right { - return performAction(record.element, "AXShowMenu") ? "AXShowMenu" : nil + return try performAction(record.element, "AXShowMenu") ? "AXShowMenu" : nil } for action in ["AXPress", "AXConfirm", "AXOpen"] { - if performAction(record.element, action) { + if try performAction(record.element, action) { return action } } @@ -793,7 +845,7 @@ final class Provider { guard let action else { throw ProviderError.coded("action_not_supported", "'\(requested)' is not a valid secondary action for element \(record.index)") } - guard performAction(record.element, action) else { + guard try performAction(record.element, action) else { throw ProviderError.coded("accessibility_error", "AXUIElementPerformAction(\(action)) failed") } return actionMetadata(path: "accessibility", actionName: action) @@ -809,6 +861,7 @@ final class Provider { let current = rawAttributeValue(record.element, kAXValueAttribute as String) let coercion = AttributeValueCoercion(existingValue: current, requested: expected) let result: AXError + let writeStart = CFAbsoluteTimeGetCurrent() switch coercion.writeValue { case .string: result = AXUIElementSetAttributeValue(record.element, kAXValueAttribute as CFString, expected as CFString) @@ -819,6 +872,9 @@ final class Provider { case let .boolean(value): result = AXUIElementSetAttributeValue(record.element, kAXValueAttribute as CFString, value ? kCFBooleanTrue : kCFBooleanFalse) } + if AccessibilityMessaging.timedOut(result, since: writeStart) { + throw AccessibilityMessaging.unanswered("the value write") + } guard result == .success else { throw ProviderError.coded("accessibility_error", "AXUIElementSetAttributeValue failed with \(result.rawValue)") } @@ -837,7 +893,7 @@ final class Provider { private func typeText(params: [String: JSONValue]) throws -> [String: Any] { let snapshot = try currentKeyboardSnapshot(params: params) let text = try requiredString(params, "text") - if let focused = focusedRecord(snapshot), let verification = TextInput.replaceSelection(focused.element, with: text) { + if let focused = focusedRecord(snapshot), let verification = try TextInput.replaceSelection(focused.element, with: text) { return actionMetadata(path: "accessibility", actionName: "AXReplaceSelection", verification: verification) } try deliverKeys(try withTransportErrors { try BackgroundInputTransport.textEvents(text) }, snapshot: snapshot, params: params) @@ -879,7 +935,7 @@ final class Provider { private func pasteText(params: [String: JSONValue]) throws -> [String: Any] { let snapshot = try currentKeyboardSnapshot(params: params) let text = try requiredString(params, "text") - if let focused = focusedRecord(snapshot), let verification = TextInput.replaceSelection(focused.element, with: text) { + if let focused = focusedRecord(snapshot), let verification = try TextInput.replaceSelection(focused.element, with: text) { return actionMetadata(path: "accessibility", actionName: "AXReplaceSelection", verification: verification) } let pasteKeys = try Input.keyEvents("cmd+v") @@ -903,7 +959,7 @@ final class Provider { if pages.rounded() == pages, let pageCount = boundedInteger(pages, as: Int.self), record.actions.contains(action) { for _ in 0.. } private func isTargetWindowFocused(_ snapshot: Snapshot) -> Bool { - guard let focusedWindow = focusedSystemWindow(systemWide: AXUIElementCreateSystemWide(), app: snapshot.app) else { - return false - } - if windowNumber(focusedWindow) == snapshot.windowId { - return true - } - guard let frame = absoluteFrame(focusedWindow) else { - return false - } - let intersection = frame.intersection(snapshot.windowBounds) - return !intersection.isNull && intersection.area >= min(frame.area, snapshot.windowBounds.area) * 0.75 + focusedSyntheticClickRecipient( + targetPID: snapshot.app.pid, + targetCandidates: { WindowCapture.candidates(pid: snapshot.app.pid) } + ) == .focused(syntheticClickRecipient(pid: snapshot.app.pid, windowId: snapshot.windowId)) } private enum AXElementProbe { @@ -1260,19 +1309,8 @@ private func focusedSyntheticClickRecipient( return .unavailable } - if let windowId = windowNumber(focusedWindow) { - return .focused(syntheticClickRecipient(pid: ownerPID, windowId: windowId)) - } - guard ownerPID == targetPID else { return .unavailable } - guard let frame = absoluteFrame(focusedWindow), - let candidate = SyntheticMouseClickDelivery.uniqueWindowCandidate( - from: targetCandidates(), - matching: { - windowFramesMatch($0.bounds, frame) - } - ) - else { return .unavailable } - return .focused(syntheticClickRecipient(pid: ownerPID, windowId: candidate.windowId)) + guard pidAttribute(focusedWindow) == ownerPID else { return .unavailable } + return pointerRecipient(of: focusedWindow, targetPID: targetPID, targetCandidates: targetCandidates) } private func hitTestSyntheticClickRecipient( @@ -1308,13 +1346,13 @@ private func pointerRecipient( guard let ownerPID = pidAttribute(element), let window = containingWindow(element) else { return .unavailable } - var numberValue: CFTypeRef? - let numberStatus = AXUIElementCopyAttributeValue(window, "AXWindowNumber" as CFString, &numberValue) - if numberStatus == .success { - guard let number = numberValue as? NSNumber else { return .unavailable } - return .focused(syntheticClickRecipient(pid: ownerPID, windowId: CGWindowID(number.uint32Value))) + do { + if let windowID = try AccessibilityWindowIdentity.windowID(of: window) { + return .focused(syntheticClickRecipient(pid: ownerPID, windowId: windowID)) + } + } catch { + return .unavailable } - guard numberStatus == .attributeUnsupported || numberStatus == .noValue else { return .unavailable } guard ownerPID == targetPID, let frame = absoluteFrame(window), let windows = copyArray(AXUIElementCreateApplication(ownerPID), kAXWindowsAttribute as String) else { return .unavailable } @@ -1723,10 +1761,7 @@ private func numberAttribute(_ element: AXUIElement, _ attribute: String) -> NSN } private func windowNumber(_ element: AXUIElement) -> CGWindowID? { - guard let number = numberAttribute(element, "AXWindowNumber") else { - return nil - } - return CGWindowID(number.uint32Value) + try? AccessibilityWindowIdentity.windowID(of: element) } private func absoluteFrame(_ element: AXUIElement) -> CGRect? { @@ -1763,9 +1798,44 @@ private func actions(_ element: AXUIElement) -> [String] { return value as? [String] ?? [] } -private func performAction(_ element: AXUIElement, _ action: String) -> Bool { - actions(element).contains(where: { $0.caseInsensitiveCompare(action) == .orderedSame }) && - AXUIElementPerformAction(element, action as CFString) == .success +private enum AccessibilityMessaging { + static let timeoutSeconds: Float = 3 + + /// Why: AX also returns cannotComplete at once for some attributes; only a call that waited out the timeout means the app is not answering. + static func timedOut(_ result: AXError, since start: CFAbsoluteTime) -> Bool { + result == .cannotComplete && CFAbsoluteTimeGetCurrent() - start >= Double(timeoutSeconds) * 0.9 + } + + static func appNotResponding(_ appName: String?) -> ProviderError { + .coded("action_timeout", "\(appName.map { "'\($0)'" } ?? "The app") is not answering accessibility requests (no reply within \(timeoutSeconds) s); it may be busy or hung. Wait until it responds, then retry.") + } + + /// The request reached the app, which went quiet while handling it, so the effect may have happened. + static func unanswered(_ request: String) -> ProviderError { + .coded("action_timeout", "the app did not answer \(request) within \(timeoutSeconds) s; it may be busy or showing a dialog, and the action may already have taken effect. Run get-app-state to check before retrying.") + } +} + +private func performAction(_ element: AXUIElement, _ action: String) throws -> Bool { + var names: CFArray? + let start = CFAbsoluteTimeGetCurrent() + let namesResult = AXUIElementCopyActionNames(element, &names) + if AccessibilityMessaging.timedOut(namesResult, since: start) { + throw AccessibilityMessaging.appNotResponding(nil) + } + guard namesResult == .success, + let names = names as? [String], + names.contains(where: { $0.caseInsensitiveCompare(action) == .orderedSame }) + else { + return false + } + let pressStart = CFAbsoluteTimeGetCurrent() + let result = AXUIElementPerformAction(element, action as CFString) + // Why: falling through to another route after a delivered but unanswered press ran the action twice. + if AccessibilityMessaging.timedOut(result, since: pressStart) { + throw AccessibilityMessaging.unanswered(action) + } + return result == .success } private func isSettable(_ element: AXUIElement, _ attribute: String) -> Bool { @@ -1889,6 +1959,8 @@ private final class TreeRenderer { static let maxNodes = 1200 static let maxDepth = 64 + var appStoppedResponding: Bool { reader.appStoppedResponding } + init(windowBounds: CGRect, focused: AXUIElement?, compactBrowserTabs: Bool) { self.windowBounds = windowBounds self.focused = focused @@ -1896,6 +1968,7 @@ private final class TreeRenderer { } func render(_ element: AXUIElement, depth: Int = 0, ancestors: [AXUIElement] = []) { + guard !reader.appStoppedResponding else { return } guard nextIndex < Self.maxNodes else { truncated = true return @@ -2096,7 +2169,7 @@ private final class AXSnapshotReader { return actions } var value: CFArray? - let actions = AXUIElementCopyActionNames(element, &value) == .success ? value as? [String] ?? [] : [] + let actions = answered { AXUIElementCopyActionNames(element, &value) } == .success ? value as? [String] ?? [] : [] cache.actions = actions return actions } @@ -2107,11 +2180,24 @@ private final class AXSnapshotReader { return cached } var settable = DarwinBoolean(false) - let value = AXUIElementIsAttributeSettable(element, attribute as CFString, &settable) == .success && settable.boolValue + let value = answered { AXUIElementIsAttributeSettable(element, attribute as CFString, &settable) } == .success && settable.boolValue cache.settable[attribute] = value return value } + /// Set once the app waits out a messaging timeout; later reads return nothing instead of each waiting it out again. + private(set) var appStoppedResponding = false + + private func answered(_ call: () -> AXError) -> AXError? { + guard !appStoppedResponding else { return nil } + let start = CFAbsoluteTimeGetCurrent() + let result = call() + if AccessibilityMessaging.timedOut(result, since: start) { + appStoppedResponding = true + } + return result + } + func frame(_ element: AXUIElement, windowBounds: CGRect) -> CGRect? { guard let absolute = absoluteFrame(element) else { return nil } return CGRect( @@ -2238,7 +2324,7 @@ private final class AXSnapshotReader { return nil } var value: CFTypeRef? - guard AXUIElementCopyAttributeValue(element, attribute as CFString, &value) == .success, let value else { + guard answered({ AXUIElementCopyAttributeValue(element, attribute as CFString, &value) }) == .success, let value else { cache.attributes[attribute] = .missing return nil } @@ -2252,7 +2338,7 @@ private final class AXSnapshotReader { } cache.loadedAttributeNames = true var value: CFArray? - guard AXUIElementCopyAttributeNames(cache.element, &value) == .success, let attributes = value as? [String] else { + guard answered({ AXUIElementCopyAttributeNames(cache.element, &value) }) == .success, let attributes = value as? [String] else { return nil } cache.advertisedAttributes = Set(attributes) @@ -2813,7 +2899,7 @@ private enum Input { } private enum TextInput { - static func replaceSelection(_ element: AXUIElement, with text: String) -> [String: Any]? { + static func replaceSelection(_ element: AXUIElement, with text: String) throws -> [String: Any]? { guard isSettable(element, kAXValueAttribute as String), let current = rawStringAttribute(element, kAXValueAttribute as String) else { @@ -2825,7 +2911,13 @@ private enum TextInput { let start = String.Index(utf16Offset: startOffset, in: current) let end = String.Index(utf16Offset: endOffset, in: current) let next = String(current[.. CGWindowID? { + var nativeAbsent = true + if let getWindow { + var windowID: CGWindowID = 0 + let status = getWindow(element, &windowID) + if status == .success, windowID != 0 { return windowID } + nativeAbsent = status == .attributeUnsupported || status == .noValue + } + + var value: CFTypeRef? + switch AXUIElementCopyAttributeValue(element, "AXWindowNumber" as CFString, &value) { + case .success: + guard let number = value as? NSNumber, + CFGetTypeID(number) != CFBooleanGetTypeID(), + let windowID = CGWindowID(exactly: number.doubleValue), windowID != 0 + else { throw Failure.unreadable } + return windowID + case .attributeUnsupported, .noValue: + guard nativeAbsent else { throw Failure.unreadable } + return nil + default: + throw Failure.unreadable + } + } + + private typealias GetWindow = @convention(c) (AXUIElement, UnsafeMutablePointer) -> AXError + + // AppKit and WebKit windows often omit AXWindowNumber. + private static let getWindow: GetWindow? = { + guard let symbol = dlsym(UnsafeMutableRawPointer(bitPattern: -2), "_AXUIElementGetWindow") else { return nil } + // SAFETY: HIServices exports AXError _AXUIElementGetWindow(AXUIElementRef, CGWindowID *). + return unsafeBitCast(symbol, to: GetWindow.self) + }() +} diff --git a/native/computer-use-macos/Sources/OrcaComputerUseMacOSCore/BackgroundInputTransport.swift b/native/computer-use-macos/Sources/OrcaComputerUseMacOSCore/BackgroundInputTransport.swift index 5c49f131599..c66fe0402ca 100644 --- a/native/computer-use-macos/Sources/OrcaComputerUseMacOSCore/BackgroundInputTransport.swift +++ b/native/computer-use-macos/Sources/OrcaComputerUseMacOSCore/BackgroundInputTransport.swift @@ -210,9 +210,8 @@ public enum BackgroundInputTransport { else { throw Failure.targetWindowNotKey } // SAFETY: the CFTypeID check above proves focused is an AXUIElement. let window = focused as! AXUIElement - var number: CFTypeRef? - guard AXUIElementCopyAttributeValue(window, "AXWindowNumber" as CFString, &number) == .success, - let number = number as? NSNumber, number.uint32Value == target.windowID + guard let windowID = try? AccessibilityWindowIdentity.windowID(of: window), + windowID == target.windowID else { throw Failure.targetWindowNotKey } try body() return diff --git a/skill-guides/computer-use.md b/skill-guides/computer-use.md index af8c7bead13..7ec6d2d92b0 100644 --- a/skill-guides/computer-use.md +++ b/skill-guides/computer-use.md @@ -87,6 +87,7 @@ printf '%s' "$TEXT" | ORCA computer set-value --app --element-index { }) }) + it('fails set-value when the provider read the write back as a different value', async () => { + const call = callComputerSidecarAction('setValue', { + app: 'Safari', + elementIndex: 15, + value: 'pf1300-awllch' + }) + const child = children[0]! + const request = child.sent[0]! + + child.emit('message', { + id: request.id, + ok: true, + result: { + snapshot: { + id: 'snap-1', + app: { name: 'Safari', bundleId: 'com.apple.Safari', pid: 100 }, + window: { title: 'Form', id: 63, width: 800, height: 600 }, + coordinateSpace: 'window', + treeText: 'tree', + elementCount: 16, + focusedElementId: null + }, + screenshot: null, + screenshotStatus: { state: 'skipped', reason: 'no_screenshot_flag' }, + action: { + path: 'accessibility', + actionName: 'AXSetValue', + fallbackReason: null, + targetWindowId: 63, + verification: { + state: 'unverified', + reason: 'value_mismatch', + expected: 'pf1300-awllch', + actualPreview: 'orig-objective-1300' + } + } + } + }) + + await expect(call).rejects.toMatchObject({ + code: 'verification_failed', + message: expect.stringContaining('"orig-objective-1300"') + }) + }) + it('rejects oversized pasteText payloads before forking the sidecar', async () => { const secret = 'sidecar-secret-token' diff --git a/src/main/computer/sidecar-client.ts b/src/main/computer/sidecar-client.ts index 57fd6689857..4a86a108632 100644 --- a/src/main/computer/sidecar-client.ts +++ b/src/main/computer/sidecar-client.ts @@ -7,7 +7,10 @@ import type { ComputerProviderCapabilities, ComputerSnapshotResult } from '../../shared/runtime-types' -import { normalizeComputerActionResult } from './computer-action-verification-normalization' +import { + normalizeComputerActionResult, + rejectContradictedComputerAction +} from './computer-action-verification-normalization' import { isComputerSidecarDiagnostic, logComputerDiagnostic } from './computer-sidecar-diagnostics' import { validateComputerSidecarPasteText } from './computer-sidecar-paste-validation' import { RuntimeClientError } from './runtime-client-error' @@ -85,8 +88,10 @@ export async function callComputerSidecarAction( if (validation) { await validation } - return normalizeComputerActionResult( - (await getComputerSidecar().call(method, params)) as ComputerActionResult + return rejectContradictedComputerAction( + normalizeComputerActionResult( + (await getComputerSidecar().call(method, params)) as ComputerActionResult + ) ) } diff --git a/src/shared/computer-use-error-recovery.ts b/src/shared/computer-use-error-recovery.ts index 86b7c18d262..43721c5d28b 100644 --- a/src/shared/computer-use-error-recovery.ts +++ b/src/shared/computer-use-error-recovery.ts @@ -90,6 +90,11 @@ export function computerUseErrorRecoveryData( 'Run `orca computer get-app-state --app --json` before retrying so you know whether the UI changed.', 'Retry with a simpler semantic action or `--no-screenshot` if observation is slow; do not repeat the same timed-out action blindly.' ) + case 'verification_failed': + return recoverWith( + 'Run `orca computer get-app-state --app --json` and check which field holds the value; another field may have changed instead.', + 'Do not retry the same `set-value` unchanged. For editable text, click the field and confirm focus, select all (`cmd+a` on macOS, `ctrl+a` on Windows/Linux), then use `type-text` and inspect the resulting value.' + ) case 'screenshot_failed': return recoverWith( 'If the accessibility tree is sufficient, rerun the command with `--no-screenshot` instead of retrying the same screenshot capture.', diff --git a/src/shared/computer-use-runtime-types.ts b/src/shared/computer-use-runtime-types.ts index e2e7cbebc4a..ff077347ae4 100644 --- a/src/shared/computer-use-runtime-types.ts +++ b/src/shared/computer-use-runtime-types.ts @@ -14,6 +14,7 @@ export const COMPUTER_ERROR_CODES = { value_not_settable: 'value_not_settable', invalid_argument: 'invalid_argument', action_timeout: 'action_timeout', + verification_failed: 'verification_failed', screenshot_failed: 'screenshot_failed', accessibility_error: 'accessibility_error' } as const @@ -112,6 +113,8 @@ export type ComputerActionVerification = | 'readback_unsupported' | 'window_changed' | 'value_mismatch' + // The accessibility call succeeded but the window and the app's windows looked the same about a second later. + | 'suspected_noop' expected?: string | null actualPreview?: string | null }