diff --git a/config/scripts/computer-use-modifier-safety.test.mjs b/config/scripts/computer-use-modifier-safety.test.mjs index 32fdd94e9..725f17fdf 100644 --- a/config/scripts/computer-use-modifier-safety.test.mjs +++ b/config/scripts/computer-use-modifier-safety.test.mjs @@ -30,7 +30,8 @@ describe('computer-use modifier safety', () => { expect(mouseInput).toContain('event.flags = flags') // Every click event flows through the shared delivery plan and carries // the modifier flags on the mouse event itself. - expect(clickInput).toContain('SyntheticMouseClickDelivery.steps(clickCount: count)') + expect(clickInput).toContain('SyntheticMouseClickDelivery.deliver(') + expect(clickInput).toContain('currentSyntheticClickRecipient(') expect(clickInput).toContain('event.flags = flags') expect(clickInput).not.toContain('down: true') }) diff --git a/native/computer-use-macos/Sources/OrcaComputerUseMacOS/main.swift b/native/computer-use-macos/Sources/OrcaComputerUseMacOS/main.swift index 7236f7bab..7ecbdbf00 100644 --- a/native/computer-use-macos/Sources/OrcaComputerUseMacOS/main.swift +++ b/native/computer-use-macos/Sources/OrcaComputerUseMacOS/main.swift @@ -729,10 +729,16 @@ final class Provider { let snapshot = try currentSnapshot(params: params) let button = params["mouseButton"]?.string ?? "left" let count = try positiveInteger(params["clickCount"]?.number, defaultValue: 1, name: "clickCount") + guard count <= SyntheticMouseClickDelivery.maxClickCount else { + throw ProviderError.coded( + "invalid_argument", + "clickCount must be at most \(SyntheticMouseClickDelivery.maxClickCount)" + ) + } let modifiers = try KeyMap.parseModifiers(params["modifiers"]?.string) // Why: agents expect a click into a target app to make the next // keyboard action safe, even when the click uses an AX action path. - recoverWindow(snapshot.app) + recoverWindow(snapshot.app, windowId: snapshot.windowId, windowBounds: snapshot.windowBounds) if let elementIndex = try optionalInteger(params, "elementIndex") { let record = try element(snapshot, elementIndex) if modifiers.isEmpty, count <= 1, let actionName = try performClickAction(record: record, mouseButton: button) { @@ -743,7 +749,8 @@ final class Provider { at: point, button: mouseButton(button), count: count, - modifiers: modifiers + modifiers: modifiers, + targetWindow: snapshot ) return actionMetadata( path: "synthetic", @@ -758,7 +765,8 @@ final class Provider { at: point, button: mouseButton(button), count: count, - modifiers: modifiers + modifiers: modifiers, + targetWindow: snapshot ) return actionMetadata( path: "synthetic", @@ -1123,6 +1131,124 @@ private func isTargetWindowFocused(_ snapshot: Snapshot) -> Bool { return !intersection.isNull && intersection.area >= min(frame.area, snapshot.windowBounds.area) * 0.75 } +private func currentSyntheticClickRecipient( + snapshot: Snapshot, + point: CGPoint +) -> SyntheticMouseClickDelivery.Recipient? { + let target = syntheticClickRecipient(pid: snapshot.app.pid, windowId: snapshot.windowId) + var cachedTargetCandidates: [WindowCandidate]? + func targetCandidates() -> [WindowCandidate] { + if let cachedTargetCandidates { return cachedTargetCandidates } + let candidates = WindowCapture.candidates(pid: snapshot.app.pid) + cachedTargetCandidates = candidates + return candidates + } + if let focused = focusedSyntheticClickRecipient( + targetPID: snapshot.app.pid, + targetCandidates: targetCandidates + ) { + guard focused == target else { return focused } + } else { + guard NSWorkspace.shared.frontmostApplication?.processIdentifier == snapshot.app.pid else { + return nil + } + } + return hitTestSyntheticClickRecipient( + at: point, + targetPID: snapshot.app.pid, + targetCandidates: targetCandidates + ) +} + +private func focusedSyntheticClickRecipient( + targetPID: pid_t, + targetCandidates: () -> [WindowCandidate] +) -> SyntheticMouseClickDelivery.Recipient? { + let systemWide = AXUIElementCreateSystemWide() + guard let focusedApp = copyElement(systemWide, kAXFocusedApplicationAttribute as String), + let ownerPID = pidAttribute(focusedApp), + let focusedWindow = copyElement(systemWide, kAXFocusedWindowAttribute as String) ?? + copyElement(focusedApp, kAXFocusedWindowAttribute as String) + else { + return nil + } + if let windowId = windowNumber(focusedWindow) { + return syntheticClickRecipient(pid: ownerPID, windowId: windowId) + } + guard ownerPID == targetPID else { return nil } + guard let frame = absoluteFrame(focusedWindow), + let candidate = SyntheticMouseClickDelivery.uniqueWindowCandidate( + from: targetCandidates(), + matching: { + windowFramesMatch($0.bounds, frame) + } + ) + else { + return nil + } + return syntheticClickRecipient(pid: ownerPID, windowId: candidate.windowId) +} + +private func hitTestSyntheticClickRecipient( + at point: CGPoint, + targetPID: pid_t, + targetCandidates: () -> [WindowCandidate] +) -> SyntheticMouseClickDelivery.Recipient? { + let systemWide = AXUIElementCreateSystemWide() + var hitElement: AXUIElement? + guard AXUIElementCopyElementAtPosition( + systemWide, + Float(point.x), + Float(point.y), + &hitElement + ) == .success, + let hitElement, + let ownerPID = pidAttribute(hitElement), + let window = containingWindow(hitElement) + else { + return nil + } + if let windowId = windowNumber(window) { + return syntheticClickRecipient(pid: ownerPID, windowId: windowId) + } + guard ownerPID == targetPID else { return nil } + guard let frame = absoluteFrame(window), + let candidate = SyntheticMouseClickDelivery.uniqueWindowCandidate( + from: targetCandidates(), + matching: { + windowFramesMatch($0.bounds, frame) + } + ) + else { + return nil + } + return syntheticClickRecipient(pid: ownerPID, windowId: candidate.windowId) +} + +private func containingWindow(_ element: AXUIElement) -> AXUIElement? { + var current = element + for _ in 0..<64 { + if stringAttribute(current, kAXRoleAttribute as String) == kAXWindowRole as String { + return current + } + if let window = copyElement(current, kAXWindowAttribute as String) { + return window + } + guard let parent = copyElement(current, kAXParentAttribute as String) else { + return nil + } + current = parent + } + return nil +} + +private func syntheticClickRecipient( + pid: pid_t, + windowId: CGWindowID +) -> SyntheticMouseClickDelivery.Recipient { + SyntheticMouseClickDelivery.Recipient(ownerPID: pid, windowID: windowId) +} + private func requireTargetWindowFocused(_ snapshot: Snapshot, restoreWindowRequested: Bool) throws { guard let failure = KeyboardInputSafety.syntheticInputFocusFailure( targetWindowFocused: isTargetWindowFocused(snapshot), @@ -1163,14 +1289,44 @@ private func matchingWindow(appElement: AXUIElement, capture: WindowCapture, foc } ?? focused } -private func recoverWindow(_ app: AppDescriptor) { +private func recoverWindow( + _ app: AppDescriptor, + windowId: CGWindowID? = nil, + windowBounds: CGRect? = nil +) { _ = app.app.unhide() _ = app.app.activate(options: [.activateAllWindows]) if let bundleId = app.bundleId { openBundle(bundleId) } let appElement = AXUIElementCreateApplication(app.pid) - if let window = copyElement(appElement, kAXFocusedWindowAttribute as String) ?? copyArray(appElement, kAXWindowsAttribute as String)?.first { + let focusedWindow = copyElement(appElement, kAXFocusedWindowAttribute as String) + var cachedWindows: [AXUIElement]? + func windows() -> [AXUIElement] { + if let cachedWindows { return cachedWindows } + let value = copyArray(appElement, kAXWindowsAttribute as String) ?? [] + cachedWindows = value + return value + } + let targetWindow: AXUIElement? + if let focusedWindow, + (windowId == nil && windowBounds == nil || windowMatchesCapture( + focusedWindow, + windowId: windowId, + windowBounds: windowBounds + )) { + targetWindow = focusedWindow + } else { + let exactWindow = windowId.flatMap { targetId in + windows().first { windowNumber($0) == targetId } + } + targetWindow = exactWindow ?? windowBounds.flatMap { targetBounds in + windows().first { window in + absoluteFrame(window).map { windowFramesMatch($0, targetBounds) } == true + } + } + } + if let window = targetWindow ?? focusedWindow ?? windows().first { _ = AXUIElementSetAttributeValue(window, kAXMinimizedAttribute as CFString, kCFBooleanFalse) _ = AXUIElementPerformAction(window, kAXRaiseAction as CFString) _ = AXUIElementSetAttributeValue(window, kAXMainAttribute as CFString, kCFBooleanTrue) @@ -1179,6 +1335,24 @@ private func recoverWindow(_ app: AppDescriptor) { Thread.sleep(forTimeInterval: 0.4) } +private func windowMatchesCapture( + _ window: AXUIElement, + windowId: CGWindowID?, + windowBounds: CGRect? +) -> Bool { + if let windowId, windowNumber(window) == windowId { return true } + guard let windowBounds, let frame = absoluteFrame(window) else { return false } + return windowFramesMatch(frame, windowBounds) +} + +private func windowFramesMatch(_ lhs: CGRect, _ rhs: CGRect) -> Bool { + let tolerance: CGFloat = 2 + return abs(lhs.minX - rhs.minX) <= tolerance && + abs(lhs.minY - rhs.minY) <= tolerance && + abs(lhs.width - rhs.width) <= tolerance && + abs(lhs.height - rhs.height) <= tolerance +} + private func openBundle(_ bundleId: String) { let process = Process() process.executableURL = URL(fileURLWithPath: "/usr/bin/open") @@ -2294,7 +2468,8 @@ private enum Input { at point: CGPoint, button: MouseButton, count: Int, - modifiers: [KeyModifier] + modifiers: [KeyModifier], + targetWindow: Snapshot ) throws { guard let source = CGEventSource(stateID: .combinedSessionState) else { throw ProviderError.coded("accessibility_error", "failed to create event source") @@ -2302,32 +2477,56 @@ private enum Input { let flags = modifiers.reduce(into: CGEventFlags()) { result, modifier in result.insert(modifier.flag) } - // Why HID tap + pacing: see SyntheticMouseClickDelivery (STA-3433). - for step in SyntheticMouseClickDelivery.steps(clickCount: count) { - let type: CGEventType - switch step { - case .move: - type = .mouseMoved - case .buttonDown: - type = button.downEvent - case .buttonUp: - type = button.upEvent + let target = syntheticClickRecipient(pid: targetWindow.app.pid, windowId: targetWindow.windowId) + do { + try SyntheticMouseClickDelivery.deliver( + clickCount: count, + target: target, + currentRecipient: { + currentSyntheticClickRecipient(snapshot: targetWindow, point: point) + }, + makeEvent: { step in + let type: CGEventType + switch step { + case .move: + type = .mouseMoved + case .buttonDown: + type = button.downEvent + case .buttonUp: + type = button.upEvent + } + guard let event = CGEvent( + mouseEventSource: source, + mouseType: type, + mouseCursorPosition: point, + mouseButton: button.cgButton + ) else { + throw ProviderError.coded("accessibility_error", "failed to create mouse event") + } + event.flags = flags + let clickState = SyntheticMouseClickDelivery.clickState(for: step) + if clickState > 0 { + event.setIntegerValueField(.mouseEventClickState, value: clickState) + } + return event + }, + post: { $0.post(tap: .cghidEventTap) }, + pause: { _ = usleep($0) } + ) + } catch let failure as SyntheticMouseClickDelivery.FenceFailure { + switch failure { + case let .recipientChanged(expected, actual, deliveredPresses): + let actualDescription = actual.map { + "pid \($0.ownerPID) window \($0.windowID)" + } ?? "no focused window" + let recovery = deliveredPresses == 0 + ? "bring the target window forward, run get-app-state again, and retry" + : "\(deliveredPresses) press(es) may already have been delivered; run get-app-state and verify state before retrying" + throw ProviderError.coded( + "window_not_focused", + "coordinate click aborted because target pid \(expected.ownerPID) window \(expected.windowID) is no longer the focused topmost recipient (current: \(actualDescription)); \(recovery)" + ) } - guard let event = CGEvent( - mouseEventSource: source, - mouseType: type, - mouseCursorPosition: point, - mouseButton: button.cgButton - ) else { - throw ProviderError.coded("accessibility_error", "failed to create mouse event") - } - event.flags = flags - let clickState = SyntheticMouseClickDelivery.clickState(for: step) - if clickState > 0 { - event.setIntegerValueField(.mouseEventClickState, value: clickState) - } - event.post(tap: .cghidEventTap) - usleep(SyntheticMouseClickDelivery.interEventPauseMicroseconds) } } diff --git a/native/computer-use-macos/Sources/OrcaComputerUseMacOSCore/SyntheticMouseClickDelivery.swift b/native/computer-use-macos/Sources/OrcaComputerUseMacOSCore/SyntheticMouseClickDelivery.swift index d61e56af3..13aa6e15c 100644 --- a/native/computer-use-macos/Sources/OrcaComputerUseMacOSCore/SyntheticMouseClickDelivery.swift +++ b/native/computer-use-macos/Sources/OrcaComputerUseMacOSCore/SyntheticMouseClickDelivery.swift @@ -6,6 +6,22 @@ /// happens). The window server also drops a mouseUp posted back-to-back /// with its mouseDown, so consecutive events need a pause between them. public enum SyntheticMouseClickDelivery { + public static let maxClickCount = 3 + + public struct Recipient: Equatable, Sendable { + public let ownerPID: Int32 + public let windowID: UInt32 + + public init(ownerPID: Int32, windowID: UInt32) { + self.ownerPID = ownerPID + self.windowID = windowID + } + } + + public enum FenceFailure: Error, Equatable { + case recipientChanged(expected: Recipient, actual: Recipient?, deliveredPresses: Int) + } + public enum Step: Equatable { case move case buttonDown(pressIndex: Int) @@ -21,7 +37,7 @@ public enum SyntheticMouseClickDelivery { /// clicks instead of independent single clicks. public static func steps(clickCount: Int) -> [Step] { var steps: [Step] = [.move] - for press in 1...max(clickCount, 1) { + for press in 1...min(max(clickCount, 1), maxClickCount) { steps.append(.buttonDown(pressIndex: press)) steps.append(.buttonUp(pressIndex: press)) } @@ -37,4 +53,52 @@ public enum SyntheticMouseClickDelivery { return Int64(pressIndex) } } + + public static func uniqueWindowCandidate( + from candidates: [Candidate], + matching predicate: (Candidate) -> Bool + ) -> Candidate? { + var match: Candidate? + for candidate in candidates where predicate(candidate) { + guard match == nil else { return nil } + match = candidate + } + return match + } + + public static func deliver( + clickCount: Int, + target: Recipient, + currentRecipient: () -> Recipient?, + makeEvent: (Step) throws -> Event, + post: (Event) -> Void, + pause: (UInt32) -> Void + ) throws { + post(try makeEvent(.move)) + pause(interEventPauseMicroseconds) + for pressIndex in 1...min(max(clickCount, 1), maxClickCount) { + let beforeDown = currentRecipient() + guard beforeDown == target else { + throw FenceFailure.recipientChanged( + expected: target, + actual: beforeDown, + deliveredPresses: pressIndex - 1 + ) + } + let down = try makeEvent(.buttonDown(pressIndex: pressIndex)) + let up = try makeEvent(.buttonUp(pressIndex: pressIndex)) + post(down) + pause(interEventPauseMicroseconds) + post(up) + let afterUp = currentRecipient() + guard afterUp == target else { + throw FenceFailure.recipientChanged( + expected: target, + actual: afterUp, + deliveredPresses: pressIndex + ) + } + pause(interEventPauseMicroseconds) + } + } } diff --git a/native/computer-use-macos/Tests/OrcaComputerUseMacOSTests/SyntheticMouseClickDeliveryTests.swift b/native/computer-use-macos/Tests/OrcaComputerUseMacOSTests/SyntheticMouseClickDeliveryTests.swift index d80a5e9e8..5870e7408 100644 --- a/native/computer-use-macos/Tests/OrcaComputerUseMacOSTests/SyntheticMouseClickDeliveryTests.swift +++ b/native/computer-use-macos/Tests/OrcaComputerUseMacOSTests/SyntheticMouseClickDeliveryTests.swift @@ -29,6 +29,18 @@ final class SyntheticMouseClickDeliveryTests: XCTestCase { } } + func testExcessiveClickCountIsCappedAtTripleClick() { + XCTAssertEqual( + SyntheticMouseClickDelivery.steps(clickCount: Int.max), + [ + .move, + .buttonDown(pressIndex: 1), .buttonUp(pressIndex: 1), + .buttonDown(pressIndex: 2), .buttonUp(pressIndex: 2), + .buttonDown(pressIndex: 3), .buttonUp(pressIndex: 3), + ] + ) + } + func testClickStateMatchesPressIndexAndSkipsMove() { XCTAssertEqual(SyntheticMouseClickDelivery.clickState(for: .move), 0) XCTAssertEqual(SyntheticMouseClickDelivery.clickState(for: .buttonDown(pressIndex: 1)), 1) @@ -40,4 +52,182 @@ final class SyntheticMouseClickDeliveryTests: XCTestCase { // turning the click into a hover-only no-op (STA-3433). XCTAssertGreaterThan(SyntheticMouseClickDelivery.interEventPauseMicroseconds, 0) } + + func testAmbiguousWindowFrameFallbackDoesNotSelectRecipient() { + let candidates = [(windowID: 101, frame: 7), (windowID: 202, frame: 7)] + + XCTAssertNil(SyntheticMouseClickDelivery.uniqueWindowCandidate( + from: candidates, + matching: { $0.frame == 7 } + )) + XCTAssertNil(SyntheticMouseClickDelivery.uniqueWindowCandidate( + from: candidates, + matching: { $0.frame == 8 } + )) + XCTAssertEqual( + SyntheticMouseClickDelivery.uniqueWindowCandidate( + from: candidates, + matching: { $0.windowID == 101 } + )?.windowID, + 101 + ) + } + + func testRecipientChangeBeforeMouseDownPostsNoClickAndReportsBothWindows() { + let target = SyntheticMouseClickDelivery.Recipient(ownerPID: 41, windowID: 101) + let intruder = SyntheticMouseClickDelivery.Recipient(ownerPID: 52, windowID: 202) + var posted: [SyntheticMouseClickDelivery.Step] = [] + + XCTAssertThrowsError( + try SyntheticMouseClickDelivery.deliver( + clickCount: 1, + target: target, + currentRecipient: { intruder }, + makeEvent: { $0 }, + post: { posted.append($0) }, + pause: { _ in } + ) + ) { error in + XCTAssertEqual( + error as? SyntheticMouseClickDelivery.FenceFailure, + .recipientChanged(expected: target, actual: intruder, deliveredPresses: 0) + ) + } + XCTAssertEqual(posted, [.move]) + } + + func testRecipientChangeAfterMouseUpStopsUntilStateIsVerified() throws { + let target = SyntheticMouseClickDelivery.Recipient(ownerPID: 41, windowID: 101) + let intruder = SyntheticMouseClickDelivery.Recipient(ownerPID: 52, windowID: 202) + var recipients = [target, intruder, target, target] + var firstAttempt: [SyntheticMouseClickDelivery.Step] = [] + + XCTAssertThrowsError( + try SyntheticMouseClickDelivery.deliver( + clickCount: 1, + target: target, + currentRecipient: { recipients.removeFirst() }, + makeEvent: { $0 }, + post: { firstAttempt.append($0) }, + pause: { _ in } + ) + ) { error in + XCTAssertEqual( + error as? SyntheticMouseClickDelivery.FenceFailure, + .recipientChanged(expected: target, actual: intruder, deliveredPresses: 1) + ) + } + XCTAssertEqual(firstAttempt, [ + .move, + .buttonDown(pressIndex: 1), + .buttonUp(pressIndex: 1), + ]) + + var retry: [SyntheticMouseClickDelivery.Step] = [] + try SyntheticMouseClickDelivery.deliver( + clickCount: 1, + target: target, + currentRecipient: { recipients.removeFirst() }, + makeEvent: { $0 }, + post: { retry.append($0) }, + pause: { _ in } + ) + XCTAssertEqual(retry, [.move, .buttonDown(pressIndex: 1), .buttonUp(pressIndex: 1)]) + } + + func testMouseUpPostsBeforeSecondRecipientCheck() throws { + let target = SyntheticMouseClickDelivery.Recipient(ownerPID: 41, windowID: 101) + var trace: [String] = [] + + try SyntheticMouseClickDelivery.deliver( + clickCount: 1, + target: target, + currentRecipient: { + trace.append("recipient") + return target + }, + makeEvent: { $0 }, + post: { + switch $0 { + case .move: + trace.append("move") + case .buttonDown: + trace.append("down") + case .buttonUp: + trace.append("up") + } + }, + pause: { _ in } + ) + + XCTAssertEqual(trace, ["move", "recipient", "down", "up", "recipient"]) + } + + func testRecipientChangeBeforeLaterPressReportsCompletedPresses() { + let target = SyntheticMouseClickDelivery.Recipient(ownerPID: 41, windowID: 101) + let intruder = SyntheticMouseClickDelivery.Recipient(ownerPID: 52, windowID: 202) + var recipients = [target, target, intruder] + var posted: [SyntheticMouseClickDelivery.Step] = [] + + XCTAssertThrowsError( + try SyntheticMouseClickDelivery.deliver( + clickCount: 2, + target: target, + currentRecipient: { recipients.removeFirst() }, + makeEvent: { $0 }, + post: { posted.append($0) }, + pause: { _ in } + ) + ) { error in + XCTAssertEqual( + error as? SyntheticMouseClickDelivery.FenceFailure, + .recipientChanged(expected: target, actual: intruder, deliveredPresses: 1) + ) + } + XCTAssertEqual(posted, [ + .move, + .buttonDown(pressIndex: 1), + .buttonUp(pressIndex: 1), + ]) + } + + func testMultiClickRevalidatesBeforeEveryPressAndAfterEveryRelease() throws { + let target = SyntheticMouseClickDelivery.Recipient(ownerPID: 41, windowID: 101) + var validationCount = 0 + var posted: [SyntheticMouseClickDelivery.Step] = [] + + try SyntheticMouseClickDelivery.deliver( + clickCount: 2, + target: target, + currentRecipient: { + validationCount += 1 + return target + }, + makeEvent: { $0 }, + post: { posted.append($0) }, + pause: { _ in } + ) + + XCTAssertEqual(validationCount, 4) + XCTAssertEqual(posted, SyntheticMouseClickDelivery.steps(clickCount: 2)) + } + + func testButtonPairIsPreparedBeforeMouseDownPosts() { + enum PreparationFailure: Error { case mouseUp } + let target = SyntheticMouseClickDelivery.Recipient(ownerPID: 41, windowID: 101) + var posted: [SyntheticMouseClickDelivery.Step] = [] + + XCTAssertThrowsError(try SyntheticMouseClickDelivery.deliver( + clickCount: 1, + target: target, + currentRecipient: { target }, + makeEvent: { step in + if case .buttonUp = step { throw PreparationFailure.mouseUp } + return step + }, + post: { posted.append($0) }, + pause: { _ in } + )) + XCTAssertEqual(posted, [.move]) + } } diff --git a/src/main/runtime/rpc/errors.test.ts b/src/main/runtime/rpc/errors.test.ts index ddd0541c5..23a6caaed 100644 --- a/src/main/runtime/rpc/errors.test.ts +++ b/src/main/runtime/rpc/errors.test.ts @@ -137,6 +137,25 @@ describe('mapRuntimeError', () => { }) }) + it('does not recommend a blind retry after a coordinate press may have landed', () => { + const message = + 'coordinate click aborted because the recipient changed; 1 press(es) may already have been delivered' + const error = Object.assign(new Error(message), { code: 'window_not_focused' }) + + const response = mapRuntimeError('req_1', { runtimeId: 'runtime-1' }, error) + + expect(response.error).toMatchObject({ + code: 'window_not_focused', + message, + data: { + nextSteps: [ + expect.stringContaining('verify whether the intended action already occurred'), + expect.stringContaining('Do not retry the click if it already took effect') + ] + } + }) + }) + it('preserves structured lineage error codes and data for CLI recovery hints', () => { const response = mapRuntimeError( 'req_1', diff --git a/src/main/runtime/rpc/errors.ts b/src/main/runtime/rpc/errors.ts index 930a056a6..a2e02e6cc 100644 --- a/src/main/runtime/rpc/errors.ts +++ b/src/main/runtime/rpc/errors.ts @@ -108,7 +108,7 @@ export function mapRuntimeError(id: string, meta: RpcEnvelopeMeta, error: unknow COMPUTER_PASSTHROUGH_CODES.has((error as { code: string }).code) ) { const code = (error as { code: string }).code - return errorResponse(id, meta, code, message, computerErrorData(code)) + return errorResponse(id, meta, code, message, computerErrorData(code, message)) } if ( error instanceof Error && diff --git a/src/shared/computer-use-error-recovery.test.ts b/src/shared/computer-use-error-recovery.test.ts index 20c870cf3..dbeaf58af 100644 --- a/src/shared/computer-use-error-recovery.test.ts +++ b/src/shared/computer-use-error-recovery.test.ts @@ -40,6 +40,19 @@ describe('computerUseErrorRecoveryData', () => { ]) }) + it('requires state verification when a focus failure may follow a delivered press', () => { + const recovery = computerUseErrorRecoveryData( + 'window_not_focused', + 'coordinate click aborted; 1 press(es) may already have been delivered' + ) + + expect(recovery?.nextSteps).toEqual([ + expect.stringContaining('verify whether the intended action already occurred'), + expect.stringContaining('Do not retry the click if it already took effect') + ]) + expect(recovery?.nextSteps.join('\n')).not.toContain('Retry once with `--restore-window`') + }) + it('keeps missing web app recovery within computer-use desktop app targeting', () => { const recovery = computerUseErrorRecoveryData('app_not_found') diff --git a/src/shared/computer-use-error-recovery.ts b/src/shared/computer-use-error-recovery.ts index 7d5434934..7803738fb 100644 --- a/src/shared/computer-use-error-recovery.ts +++ b/src/shared/computer-use-error-recovery.ts @@ -3,7 +3,8 @@ export type ComputerUseErrorRecoveryData = { } export function computerUseErrorRecoveryData( - code: string + code: string, + message?: string ): ComputerUseErrorRecoveryData | undefined { switch (code) { case 'app_not_found': @@ -24,6 +25,12 @@ export function computerUseErrorRecoveryData( 'If no window is listed, open or focus the app first; `orca computer` does not launch closed desktop apps.' ) case 'window_not_focused': + if (message?.includes('may already have been delivered')) { + return recoverWith( + 'Run `orca computer get-app-state --app --json` and verify whether the intended action already occurred before retrying.', + 'Do not retry the click if it already took effect; otherwise bring the target window forward and use fresh state before trying again.' + ) + } return recoverWith( 'Retry once with `--restore-window`.', 'If `--restore-window` was already used, stop retrying restore; bring the app forward manually, check permissions, or prefer `set-value` for editable fields.' diff --git a/tests/e2e/computer-mac.e2e.ts b/tests/e2e/computer-mac.e2e.ts index 6aa162cee..5c6d4e381 100644 --- a/tests/e2e/computer-mac.e2e.ts +++ b/tests/e2e/computer-mac.e2e.ts @@ -13,6 +13,10 @@ import { parseJsonOutput, runOrcaCli } from './helpers/computer-driver' +import { + clickCapturedTextEditOpenDialog, + doubleClickTextEditWord +} from './helpers/computer-coordinate-click-driver' const isMac = process.platform === 'darwin' const e2eOptIn = process.env.ORCA_COMPUTER_E2E === '1' @@ -72,81 +76,22 @@ describe.skipIf(!isMac || !e2eOptIn)('computer-use macOS e2e (TextEdit)', () => }) test('coordinate double-click activates a control, not just hover (STA-3433)', async () => { - // Ten identical short lines: any first-lines click hits a word, and the - // whole document stays inside the treeText value preview after editing. - const filler = Array(10).fill('wordword').join('\n') - await runOrcaCli([ - 'computer', - 'hotkey', - '--app', - 'TextEdit', - '--key', - 'CmdOrCtrl+A', - '--no-screenshot' - ]) - await runOrcaCli([ - 'computer', - 'paste-text', - '--app', - 'TextEdit', - '--text', - filler, - '--no-screenshot' - ]) + const result = await doubleClickTextEditWord() - // Word-select via coordinates: 40px in, 70px down lands inside a - // "wordword" on one of the first lines whether or not the ruler is shown. - const clicked = parseJsonOutput<{ result: ComputerActionResult }>( - ( - await runOrcaCli([ - 'computer', - 'click', - '--app', - 'TextEdit', - '--x', - '40', - '--y', - '70', - '--click-count', - '2', - '--no-screenshot', - '--json' - ]) - ).stdout - ) - expect(clicked.result.action?.path).toBe('synthetic') - expect(clicked.result.action?.verification).toMatchObject({ + expect(result.action?.path).toBe('synthetic') + expect(result.action?.verification).toMatchObject({ state: 'unverified', reason: 'synthetic_input' }) + expect(result.replacedWord).toBe(true) + }) - const marker = `zz${Date.now()}zz` - await runOrcaCli([ - 'computer', - 'type-text', - '--app', - 'TextEdit', - '--text', - marker, - '--no-screenshot' - ]) - - const after = parseJsonOutput<{ result: ComputerSnapshotResult }>( - ( - await runOrcaCli([ - 'computer', - 'get-app-state', - '--app', - 'TextEdit', - '--no-screenshot', - '--json' - ]) - ).stdout - ) - // The double-click selected a mid-document word, so the typed marker must - // be followed by more filler. A dropped press leaves the caret at the end - // of the document and the marker only ever appends (the STA-3433 no-op). - expect(after.result.snapshot.treeText).toMatch(new RegExp(`${marker}\\s+wordword`)) + test('coordinate click reaches the captured native dialog window', async () => { + expect(await clickCapturedTextEditOpenDialog()).toMatchObject({ + clickPath: 'synthetic', + dialogClosed: true, + dialogWasNew: true + }) }) test('paste-text and hotkey verify TextEdit text replacement', async () => { diff --git a/tests/e2e/helpers/computer-coordinate-click-driver.ts b/tests/e2e/helpers/computer-coordinate-click-driver.ts new file mode 100644 index 000000000..6b17e7bca --- /dev/null +++ b/tests/e2e/helpers/computer-coordinate-click-driver.ts @@ -0,0 +1,131 @@ +import type { + ComputerActionResult, + ComputerSnapshotResult +} from '../../../src/shared/runtime-types' +import { parseJsonOutput, runOrcaCli } from './computer-driver' + +export async function doubleClickTextEditWord(): Promise<{ + action: ComputerActionResult['action'] + replacedWord: boolean +}> { + const filler = Array(10).fill('wordword').join('\n') + await runOrcaCli([ + 'computer', + 'hotkey', + '--app', + 'TextEdit', + '--key', + 'CmdOrCtrl+A', + '--no-screenshot' + ]) + await runOrcaCli([ + 'computer', + 'paste-text', + '--app', + 'TextEdit', + '--text', + filler, + '--no-screenshot' + ]) + + const clicked = parseJsonOutput<{ result: ComputerActionResult }>( + ( + await runOrcaCli([ + 'computer', + 'click', + '--app', + 'TextEdit', + '--x', + '40', + '--y', + '70', + '--click-count', + '2', + '--no-screenshot', + '--json' + ]) + ).stdout + ) + const marker = `zz${Date.now()}zz` + await runOrcaCli([ + 'computer', + 'type-text', + '--app', + 'TextEdit', + '--text', + marker, + '--no-screenshot' + ]) + + const after = parseJsonOutput<{ result: ComputerSnapshotResult }>( + ( + await runOrcaCli([ + 'computer', + 'get-app-state', + '--app', + 'TextEdit', + '--no-screenshot', + '--json' + ]) + ).stdout + ) + return { + action: clicked.result.action, + replacedWord: new RegExp(`${marker}\\s+wordword`).test(after.result.snapshot.treeText) + } +} + +export async function clickCapturedTextEditOpenDialog(): Promise<{ + clickPath: string | undefined + dialogClosed: boolean + dialogWasNew: boolean +}> { + const before = parseJsonOutput<{ + result: { windows: { id?: number | null }[] } + }>((await runOrcaCli(['computer', 'list-windows', '--app', 'TextEdit', '--json'])).stdout) + const existingWindowIds = new Set(before.result.windows.map((window) => window.id)) + + const opened = parseJsonOutput<{ result: ComputerActionResult }>( + ( + await runOrcaCli([ + 'computer', + 'hotkey', + '--app', + 'TextEdit', + '--key', + 'CmdOrCtrl+O', + '--restore-window', + '--no-screenshot', + '--json' + ]) + ).stdout + ) + const dialog = opened.result.snapshot.window + const clicked = parseJsonOutput<{ result: ComputerActionResult }>( + ( + await runOrcaCli([ + 'computer', + 'click', + '--app', + 'TextEdit', + '--window-id', + String(dialog.id), + '--x', + String(dialog.width - 140), + '--y', + String(dialog.height - 30), + '--no-screenshot', + '--json' + ]) + ).stdout + ) + const after = parseJsonOutput<{ + result: { windows: { id?: number | null }[] } + }>((await runOrcaCli(['computer', 'list-windows', '--app', 'TextEdit', '--json'])).stdout) + + return { + clickPath: clicked.result.action?.path, + dialogClosed: !after.result.windows.some((window) => window.id === dialog.id), + dialogWasNew: !existingWindowIds.has(dialog.id) + } +}