diff --git a/apps/macos/Sources/OpenClaw/ComputerActionService.swift b/apps/macos/Sources/OpenClaw/ComputerActionService.swift index 3af35c915c0..a5cd1a412d3 100644 --- a/apps/macos/Sources/OpenClaw/ComputerActionService.swift +++ b/apps/macos/Sources/OpenClaw/ComputerActionService.swift @@ -486,7 +486,7 @@ final class ComputerActionService { case .eventCreationFailed: "Failed to synthesize input event" case .lifecycleChanged: - "Computer control lifecycle changed while the action was pending" + "COMPUTER_STALE_OBSERVATION: provider generation changed; take a fresh observation and retry" case let .invalidV2Request(message): "COMPUTER_INVALID_REQUEST: \(message)" case .staleObservation: diff --git a/apps/macos/Sources/OpenClaw/ComputerActionServiceV2.swift b/apps/macos/Sources/OpenClaw/ComputerActionServiceV2.swift index 11ec5834fa4..7381d81edef 100644 --- a/apps/macos/Sources/OpenClaw/ComputerActionServiceV2.swift +++ b/apps/macos/Sources/OpenClaw/ComputerActionServiceV2.swift @@ -47,17 +47,17 @@ struct ComputerActionExecutionAuthority { /// execution-local, and invalidated when the native lifecycle generation moves. @MainActor final class ComputerActionServiceV2 { - private struct WindowTarget { + struct WindowTarget { let app: ServiceApplicationInfo let window: ServiceWindowInfo } - private struct ElementTarget { + struct ElementTarget { let id: String let bounds: CGRect } - private struct ObservationState { + struct ObservationState { let id: String let windowRef: String let snapshotId: String @@ -190,7 +190,6 @@ final class ComputerActionServiceV2 { let appOutput = try await self.withExecutionAuthority { try await self.applications.listApplications() } - self.windowRefs.removeAll(keepingCapacity: true) var rows: [[String: Any]] = [] var warnings = appOutput.metadata.warnings outer: for app in appOutput.data.applications @@ -204,9 +203,8 @@ final class ComputerActionServiceV2 { } warnings.append(contentsOf: output.metadata.warnings) for window in output.data.windows where window.layer == 0 { - let ref = self.issueWindowRef(app: app, window: window) rows.append([ - "windowRef": ref, + "windowRef": self.issueWindowRef(app: app, window: window), "appName": app.name, "title": window.title, "bounds": Self.boundsDictionary(window.bounds), @@ -298,28 +296,23 @@ final class ComputerActionServiceV2 { let detected = result.elements?.elements.all ?? [] let filtered = Self.filterElements(detected, query: params.query) let bounded = Array(filtered.prefix(limits.maxElements)) - let observationID = self.issueRef("observation") - var elementTargets: [String: ElementTarget] = [:] - let elements = bounded.map { element in - let ref = self.issueRef("element") - elementTargets[ref] = ElementTarget(id: element.id, bounds: element.bounds) - return OpenClawComputerObservationElement( + let snapshotID = result.elements?.snapshotId ?? "" + guard !snapshotID.isEmpty else { + throw ComputerActionService.ComputerActionError.refused( + "Peekaboo observation returned no snapshot receipt") + } + let issued = self.issueObservation( + windowRef: windowRef, + snapshotId: snapshotID, + elements: bounded.map { ElementTarget(id: $0.id, bounds: $0.bounds) }) + let elements = zip(bounded, issued.elementRefs).map { element, ref in + OpenClawComputerObservationElement( elementRef: ref, role: element.type.rawValue, label: element.label, value: element.value, bounds: Self.bounds(element.bounds)) } - let snapshotID = result.elements?.snapshotId ?? "" - guard !snapshotID.isEmpty else { - throw ComputerActionService.ComputerActionError.refused( - "Peekaboo observation returned no snapshot receipt") - } - self.observation = ObservationState( - id: observationID, - windowRef: windowRef, - snapshotId: snapshotID, - elements: elementTargets) var details: [String: AnyCodable] = [ "totalElementCount": AnyCodable(detected.count), "coordinateSpace": AnyCodable("global-logical-points"), @@ -343,7 +336,7 @@ final class ComputerActionServiceV2 { format: "png", width: Int(size.width), height: Int(size.height), - observationId: observationID, + observationId: issued.id, elements: elements.isEmpty ? nil : elements), details: details) } @@ -603,7 +596,7 @@ final class ComputerActionServiceV2 { // MARK: - Reference and target helpers - private func adoptLifecycleGeneration(_ generation: UInt64) { + func adoptLifecycleGeneration(_ generation: UInt64) { guard self.lifecycleGeneration != generation else { return } self.lifecycleGeneration = generation self.appRefs.removeAll() @@ -611,40 +604,46 @@ final class ComputerActionServiceV2 { self.observation = nil } - private func issueRef(_ kind: String) -> String { - "peekaboo:v2:\(kind):\(self.executionID):\(self.lifecycleGeneration ?? 0):" + - UUID().uuidString.lowercased() - } - - private func issueWindowRef(app: ServiceApplicationInfo, window: ServiceWindowInfo) -> String { - if let existing = self.windowRefs.first(where: { - $0.value.app.processIdentifier == app.processIdentifier && - $0.value.window.windowID == window.windowID && - $0.value.window.mutationIdentity == window.mutationIdentity - })?.key { - return existing - } - let ref = self.issueRef("window") + /// A window ref names one live window for the whole lifecycle generation and + /// keys on stable identity only: WindowServer id plus the owner process + /// generation that guards pid reuse. Bounds and minimized state stay out — + /// they belong to the per-action expected-identity check — so a window that + /// moves, resizes, or minimizes keeps its ref and has its stored target + /// refreshed here, and later checks compare against the live window. + func issueWindowRef(app: ServiceApplicationInfo, window: ServiceWindowInfo) -> String { + let ref = self.windowRefs.first { Self.sameWindow($0.value.window, window) }?.key + ?? self.issueRef("window") self.windowRefs[ref] = WindowTarget(app: app, window: window) return ref } - private func resolveWindow(_ ref: String) throws -> WindowTarget { + func resolveWindow(_ ref: String) throws -> WindowTarget { guard let target = self.windowRefs[ref] else { throw ComputerActionService.ComputerActionError.staleObservation } return target } - private func requiredWindow(_ params: OpenClawComputerActParams) throws -> WindowTarget { - try self.resolveWindow(Self.require(params.windowRef, field: "windowRef")) + /// Only the newest observation may authorize element work, so issuing one + /// supersedes every element ref handed out by the previous observation. + func issueObservation( + windowRef: String, + snapshotId: String, + elements: [ElementTarget]) -> (id: String, elementRefs: [String]) + { + let id = self.issueRef("observation") + let elementRefs = elements.map { _ in self.issueRef("element") } + self.observation = ObservationState( + id: id, + windowRef: windowRef, + snapshotId: snapshotId, + elements: Dictionary(uniqueKeysWithValues: zip(elementRefs, elements))) + return (id, elementRefs) } - private func requiredObservation(_ params: OpenClawComputerActParams, windowRef: String) throws - -> ObservationState - { + func resolveObservation(_ id: String?, windowRef: String) throws -> ObservationState { guard let observation = self.observation, - observation.id == params.observationId, + observation.id == id, observation.windowRef == windowRef else { throw ComputerActionService.ComputerActionError.staleObservation @@ -652,23 +651,41 @@ final class ComputerActionServiceV2 { return observation } + func resolveElement(_ ref: String, observation: ObservationState) throws -> ElementTarget { + guard let element = observation.elements[ref] else { + throw ComputerActionService.ComputerActionError.staleObservation + } + return element + } + + private func issueRef(_ kind: String) -> String { + "peekaboo:v2:\(kind):\(self.executionID):\(self.lifecycleGeneration ?? 0):" + + UUID().uuidString.lowercased() + } + + private static func sameWindow(_ lhs: ServiceWindowInfo, _ rhs: ServiceWindowInfo) -> Bool { + guard let left = lhs.mutationIdentity, let right = rhs.mutationIdentity else { return false } + return left.windowID == right.windowID && left.processIdentity == right.processIdentity + } + + private func requiredWindow(_ params: OpenClawComputerActParams) throws -> WindowTarget { + try self.resolveWindow(Self.require(params.windowRef, field: "windowRef")) + } + private func requiredElement( _ params: OpenClawComputerActParams, windowRef: String) throws -> ElementTarget { let elementRef = try Self.require(params.elementRef, field: "elementRef") - let observation = try self.requiredObservation(params, windowRef: windowRef) - guard let element = observation.elements[elementRef] else { - throw ComputerActionService.ComputerActionError.staleObservation - } - return element + let observation = try self.resolveObservation(params.observationId, windowRef: windowRef) + return try self.resolveElement(elementRef, observation: observation) } private func clickTarget( _ params: OpenClawComputerActParams, windowRef: String) throws -> (target: ClickTarget, point: CGPoint, snapshotId: String) { - let observation = try self.requiredObservation(params, windowRef: windowRef) + let observation = try self.resolveObservation(params.observationId, windowRef: windowRef) if params.elementRef != nil { let element = try self.requiredElement(params, windowRef: windowRef) return (.elementId(element.id), element.bounds.centerPoint, observation.snapshotId) @@ -701,7 +718,7 @@ final class ComputerActionServiceV2 { throw ComputerActionService.ComputerActionError.invalidV2Request( "coordinates must be nonnegative") } - _ = try self.requiredObservation(params, windowRef: windowRef) + _ = try self.resolveObservation(params.observationId, windowRef: windowRef) return CGPoint(x: x, y: y) } @@ -712,10 +729,8 @@ final class ComputerActionServiceV2 { foreground: Bool) async throws -> OpenClawComputerActResult { let windowRef = try Self.require(params.windowRef, field: "windowRef") - let observation = try self.requiredObservation(params, windowRef: windowRef) - guard let element = observation.elements[elementRef] else { - throw ComputerActionService.ComputerActionError.staleObservation - } + let observation = try self.resolveObservation(params.observationId, windowRef: windowRef) + let element = try self.resolveElement(elementRef, observation: observation) if foreground { try await self.focus(target) try Self.postForegroundClick(at: element.bounds.centerPoint, action: .leftClick, modifiers: nil) diff --git a/apps/macos/Tests/OpenClawIPCTests/ComputerRefLifecycleContractTests.swift b/apps/macos/Tests/OpenClawIPCTests/ComputerRefLifecycleContractTests.swift new file mode 100644 index 00000000000..8e886bdd292 --- /dev/null +++ b/apps/macos/Tests/OpenClawIPCTests/ComputerRefLifecycleContractTests.swift @@ -0,0 +1,160 @@ +import CoreGraphics +import Foundation +import OpenClawKit +import PeekabooAutomationKit +import Testing +@testable import OpenClaw + +/// Drives the real `ComputerActionServiceV2` reference lifecycle through the same +/// case table the CUA provider runs in `ref-lifecycle.contract.test.ts`, so the two +/// providers cannot drift apart on what an opaque ref means. +@MainActor +struct ComputerRefLifecycleContractTests { + private struct Contract: Decodable { + let staleErrorCode: String + let cases: [Case] + } + + private struct Case: Decodable { + let id: String + let scenario: Scenario + let expected: Expected + } + + private enum Scenario: String, Decodable { + case freshWindow = "fresh_window" + case freshElement = "fresh_element" + case windowMoved = "window_moved" + case generationRotation = "generation_rotation" + case inFlightGenerationChange = "in_flight_generation_change" + case supersededObservation = "superseded_observation" + case unrelatedDiscovery = "unrelated_discovery" + case unknownRef = "unknown_ref" + } + + private enum Expected: String, Decodable { + case valid + case stale + } + + private static let app = ServiceApplicationInfo( + processIdentifier: 4321, + processStartIdentity: 90210, + bundleIdentifier: "com.example.contract", + name: "Contract", + windowCount: 1) + + private static let originalBounds = CGRect(x: 0, y: 0, width: 400, height: 300) + private static let movedBounds = CGRect(x: 120, y: 80, width: 640, height: 480) + + @Test func `Peekaboo satisfies the shared ref lifecycle cases`() async throws { + let contract = try Self.loadContract() + + for testCase in contract.cases { + let error = await self.run(testCase) + switch testCase.expected { + case .valid: + #expect(error == nil, "\(testCase.id) unexpectedly failed: \(String(describing: error))") + case .stale: + #expect( + error?.localizedDescription.hasPrefix("\(contract.staleErrorCode):") == true, + "\(testCase.id) returned \(String(describing: error))") + } + } + } + + private func run(_ testCase: Case) async -> Error? { + let service = ComputerActionServiceV2() + service.adoptLifecycleGeneration(1) + let windowRef = service.issueWindowRef( + app: Self.app, + window: Self.window(windowID: 77, bounds: Self.originalBounds)) + let observation = service.issueObservation( + windowRef: windowRef, + snapshotId: "snapshot-1", + elements: [ComputerActionServiceV2.ElementTarget(id: "button", bounds: .zero)]) + + do { + switch testCase.scenario { + case .freshWindow: + _ = try service.resolveWindow(windowRef) + case .freshElement: + let current = try service.resolveObservation(observation.id, windowRef: windowRef) + _ = try service.resolveElement(observation.elementRefs[0], observation: current) + case .windowMoved: + let moved = Self.window(windowID: 77, bounds: Self.movedBounds) + #expect(service.issueWindowRef(app: Self.app, window: moved) == windowRef) + let refreshed = try service.resolveWindow(windowRef).window + #expect(refreshed.bounds == Self.movedBounds) + #expect(refreshed.mutationIdentity == moved.mutationIdentity) + case .generationRotation: + service.adoptLifecycleGeneration(2) + _ = try service.resolveWindow(windowRef) + case .inFlightGenerationChange: + try await Self.performWithRevokedLifecycle(service) + case .supersededObservation: + _ = service.issueObservation( + windowRef: windowRef, + snapshotId: "snapshot-2", + elements: [ComputerActionServiceV2.ElementTarget(id: "button", bounds: .zero)]) + _ = try service.resolveObservation(observation.id, windowRef: windowRef) + case .unrelatedDiscovery: + _ = service.issueWindowRef( + app: Self.app, + window: Self.window(windowID: 88, bounds: Self.movedBounds)) + _ = try service.resolveWindow(windowRef) + case .unknownRef: + _ = try service.resolveWindow("peekaboo:v2:window:unknown") + } + return nil + } catch { + return error + } + } + + /// Runs a real action whose lifecycle generation is revoked after the work + /// completed but before the result is released, which is the only way the + /// in-flight change reaches a caller. + private static func performWithRevokedLifecycle(_ service: ComputerActionServiceV2) async throws { + var checks = 0 + _ = try await service.perform( + OpenClawComputerActParams(action: .getCursorPosition), + lifecycleGeneration: 1, + checkExecutionAllowed: { + checks += 1 + if checks > 1 { + throw ComputerActionService.ComputerActionError.lifecycleChanged + } + }) + } + + private static func window(windowID: Int, bounds: CGRect) -> ServiceWindowInfo { + ServiceWindowInfo( + windowID: windowID, + title: "Contract Window", + bounds: bounds, + mutationIdentity: WindowMutationIdentity( + windowID: windowID, + ownerProcessIdentifier: self.app.processIdentifier, + ownerProcessStartIdentity: 90210, + capturedBounds: bounds)) + } + + private static func loadContract() throws -> Contract { + var cursor = URL(fileURLWithPath: #filePath).deletingLastPathComponent() + for _ in 0..<8 { + let candidate = cursor + .appendingPathComponent("test") + .appendingPathComponent("fixtures") + .appendingPathComponent("computer-ref-lifecycle-contract.json") + if FileManager.default.fileExists(atPath: candidate.path) { + return try JSONDecoder().decode(Contract.self, from: Data(contentsOf: candidate)) + } + cursor.deleteLastPathComponent() + } + throw NSError( + domain: "ComputerRefLifecycleContractTests", + code: 1, + userInfo: [NSLocalizedDescriptionKey: "missing shared computer ref lifecycle fixture"]) + } +} diff --git a/apps/macos/Tests/OpenClawIPCTests/MacNodeRuntimeTests.swift b/apps/macos/Tests/OpenClawIPCTests/MacNodeRuntimeTests.swift index 2c9b90403e9..30ff68b1990 100644 --- a/apps/macos/Tests/OpenClawIPCTests/MacNodeRuntimeTests.swift +++ b/apps/macos/Tests/OpenClawIPCTests/MacNodeRuntimeTests.swift @@ -830,7 +830,7 @@ struct MacNodeRuntimeTests { #expect(!response.ok) #expect(response.error?.code == .unavailable) - #expect(response.error?.message.contains("lifecycle changed") == true) + #expect(response.error?.message.contains("COMPUTER_STALE_OBSERVATION") == true) #expect(generations.perform == [0]) #expect(generations.release == [1]) } diff --git a/extensions/cua-computer/src/ref-lifecycle.contract.test.ts b/extensions/cua-computer/src/ref-lifecycle.contract.test.ts new file mode 100644 index 00000000000..01b2c98c69c --- /dev/null +++ b/extensions/cua-computer/src/ref-lifecycle.contract.test.ts @@ -0,0 +1,110 @@ +import { readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; +import { driver, result } from "./commands.test-helpers.js"; +import { callWindowTool } from "./driver-result.js"; +import { + adoptGeneration, + issueElementRef, + issueObservation, + issueWindowRef, + resolveElementRef, + resolveObservation, + resolveWindowRef, + type CuaFrameState, +} from "./frame.js"; + +type RefLifecycleCase = { + id: string; + scenario: + | "fresh_window" + | "fresh_element" + | "window_moved" + | "generation_rotation" + | "in_flight_generation_change" + | "superseded_observation" + | "unrelated_discovery" + | "unknown_ref"; + expected: "valid" | "stale"; +}; + +type RefLifecycleContract = { + staleErrorCode: string; + cases: RefLifecycleCase[]; +}; + +const contract = JSON.parse( + readFileSync( + new URL("../../../test/fixtures/computer-ref-lifecycle-contract.json", import.meta.url), + "utf8", + ), +) as RefLifecycleContract; + +async function runCase(testCase: RefLifecycleCase): Promise { + const state: CuaFrameState = { generation: "generation-1" }; + const windowRef = issueWindowRef(state, { pid: 100, windowId: 10 }); + const observation = issueObservation(state, windowRef); + const elementRef = issueElementRef(observation, { elementIndex: 0 }); + + switch (testCase.scenario) { + case "fresh_window": + resolveWindowRef(state, windowRef); + return; + case "fresh_element": + resolveElementRef(resolveObservation(state, observation.id, windowRef), elementRef); + return; + case "window_moved": { + // A moved window is rediscovered under the same stable identity, so the + // ref survives and still resolves to the live window. CUA stores only that + // identity, so the refreshed target is the identity itself. + const moved = { pid: 100, windowId: 10 }; + expect(issueWindowRef(state, moved)).toBe(windowRef); + expect(resolveWindowRef(state, windowRef)).toEqual(moved); + return; + } + case "generation_rotation": + adoptGeneration(state, "generation-2"); + resolveWindowRef(state, windowRef); + return; + case "in_flight_generation_change": { + // The production path is callWindowTool: it snapshots the driver + // generation, awaits the driver call, then detects rotation. Asserting a + // pre-call guard instead would leave that post-await check unprotected. + const session = driver(); + session.callTool.mockImplementationOnce(async () => { + session.setGeneration("execution-2"); + return result({}); + }); + await callWindowTool( + session.session, + { generation: session.session.generation }, + "get_window_state", + {}, + ); + return; + } + case "superseded_observation": + issueObservation(state, windowRef); + resolveObservation(state, observation.id, windowRef); + return; + case "unrelated_discovery": + issueWindowRef(state, { pid: 200, windowId: 20 }); + resolveWindowRef(state, windowRef); + return; + case "unknown_ref": + resolveWindowRef(state, "cua:v2:window:unknown"); + } +} + +describe("Computer Use ref lifecycle contract", () => { + for (const testCase of contract.cases) { + it(testCase.id, async () => { + if (testCase.expected === "valid") { + await expect(runCase(testCase)).resolves.toBeUndefined(); + } else { + await expect(runCase(testCase)).rejects.toThrow( + new RegExp(`^${contract.staleErrorCode}:`, "u"), + ); + } + }); + } +}); diff --git a/src/plugins/computer-use-contract.test.ts b/src/plugins/computer-use-contract.test.ts index 66c72781813..136ac911c56 100644 --- a/src/plugins/computer-use-contract.test.ts +++ b/src/plugins/computer-use-contract.test.ts @@ -1,5 +1,7 @@ +import { readFileSync } from "node:fs"; import { describe, expect, it, vi } from "vitest"; import { + COMPUTER_STALE_OBSERVATION, COMPUTER_USE_V2_ACTION_NAMES, parseComputerActParamsJSON, parseComputerActResult, @@ -11,6 +13,17 @@ import { import type { OpenClawPluginNodeHostCommand } from "./types.js"; describe("Computer Use wire contract", () => { + it("owns the shared provider ref-lifecycle error code", () => { + const contract = JSON.parse( + readFileSync( + new URL("../../test/fixtures/computer-ref-lifecycle-contract.json", import.meta.url), + "utf8", + ), + ) as { staleErrorCode: string }; + + expect(contract.staleErrorCode).toBe(COMPUTER_STALE_OBSERVATION); + }); + it("validates the canonical computer.act payload", () => { expect( parseComputerActParamsJSON( diff --git a/test/fixtures/computer-ref-lifecycle-contract.json b/test/fixtures/computer-ref-lifecycle-contract.json new file mode 100644 index 00000000000..0897c791f68 --- /dev/null +++ b/test/fixtures/computer-ref-lifecycle-contract.json @@ -0,0 +1,45 @@ +{ + "staleErrorCode": "COMPUTER_STALE_OBSERVATION", + "cases": [ + { + "id": "fresh-window-ref-valid", + "scenario": "fresh_window", + "expected": "valid" + }, + { + "id": "fresh-element-ref-valid", + "scenario": "fresh_element", + "expected": "valid" + }, + { + "id": "window-moved-keeps-ref-and-refreshes-target", + "scenario": "window_moved", + "expected": "valid" + }, + { + "id": "generation-rotation-invalidates-refs", + "scenario": "generation_rotation", + "expected": "stale" + }, + { + "id": "in-flight-generation-change-uses-contract-code", + "scenario": "in_flight_generation_change", + "expected": "stale" + }, + { + "id": "superseded-observation-invalidates-elements", + "scenario": "superseded_observation", + "expected": "stale" + }, + { + "id": "unrelated-discovery-preserves-window-ref", + "scenario": "unrelated_discovery", + "expected": "valid" + }, + { + "id": "unknown-ref-uses-contract-code", + "scenario": "unknown_ref", + "expected": "stale" + } + ] +}