From 6ace7f865b4eef992c5c852c89904f84b5831230 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A8=8B=E5=BA=8F=E5=91=98=E9=98=BF=E6=B1=9F=28Relakkes?= =?UTF-8?q?=29?= Date: Wed, 26 Aug 2026 20:38:47 +0800 Subject: [PATCH] fix(computer-use): restore background macOS automation --- .../src/pages/ComputerUseSettings.test.tsx | 56 +++- desktop/src/pages/ComputerUseSettings.tsx | 105 ++++--- .../Sources/cu-helper/AXAction.swift | 10 +- .../Sources/cu-helper/CommandRouter.swift | 77 +----- .../cu-helper/SyntheticWindowFocus.swift | 207 +++++++++++++- .../cu-helper/TargetVisibilityPolicy.swift | 170 ++---------- .../SyntheticWindowFocusTests.swift | 259 ++++++++++++++++-- .../CuHelperTests/TargetVisibilityTests.swift | 243 +++++----------- 8 files changed, 646 insertions(+), 481 deletions(-) diff --git a/desktop/src/pages/ComputerUseSettings.test.tsx b/desktop/src/pages/ComputerUseSettings.test.tsx index 7a51c678..05330244 100644 --- a/desktop/src/pages/ComputerUseSettings.test.tsx +++ b/desktop/src/pages/ComputerUseSettings.test.tsx @@ -1,5 +1,5 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -import { act, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { act, fireEvent, render, screen, waitFor, within } from '@testing-library/react' import '@testing-library/jest-dom' import { ComputerUseSettings } from './ComputerUseSettings' @@ -316,6 +316,54 @@ describe('ComputerUseSettings', () => { }, } + it('does not flash the compatibility toggle before rendering the native header toggle', async () => { + const statusRequest = deferred() + computerUseApiMock.getStatus.mockReturnValue(statusRequest.promise) + + render() + + expect(screen.getByRole('status')).toHaveTextContent('Loading...') + expect(screen.queryByLabelText('Enabled')).not.toBeInTheDocument() + + await act(async () => { + statusRequest.resolve(nativeStatus) + await statusRequest.promise + }) + + const heading = await screen.findByRole('heading', { name: 'Computer Control' }) + expect(within(heading.parentElement!.parentElement!).getByRole('switch', { name: 'Enabled' })).toBeChecked() + expect(screen.queryByRole('switch', { name: 'Any App' })).not.toBeInTheDocument() + }) + + it('does not render the compatibility page when the capability probe fails', async () => { + computerUseApiMock.getStatus.mockRejectedValue(new Error('offline')) + + render() + + expect(await screen.findByText('Failed to check status.')).toBeInTheDocument() + expect(screen.queryByLabelText('Enabled')).not.toBeInTheDocument() + expect(screen.queryByRole('heading', { name: 'Computer Control' })).not.toBeInTheDocument() + }) + + it('waits for persisted config before showing the native toggle state', async () => { + const configRequest = deferred() + computerUseApiMock.getStatus.mockResolvedValue(nativeStatus) + computerUseApiMock.getAuthorizedApps.mockReturnValue(configRequest.promise) + + render() + + await waitFor(() => expect(computerUseApiMock.getStatus).toHaveBeenCalled()) + expect(screen.getByRole('status')).toHaveTextContent('Loading...') + expect(screen.queryByLabelText('Enabled')).not.toBeInTheDocument() + + await act(async () => { + configRequest.resolve({ ...enabledConfig, enabled: false }) + await configRequest.promise + }) + + expect(await screen.findByRole('switch', { name: 'Enabled' })).not.toBeChecked() + }) + /** * Rows show the application's own icon, served per bundle id. The letter * tile is the fallback for bundles that ship no icon, so it must appear on @@ -382,7 +430,7 @@ describe('ComputerUseSettings', () => { }) }) - it('pops the native permission card when enabling Any App with missing permissions', async () => { + it('pops the native permission card when enabling Computer Use with missing permissions', async () => { computerUseApiMock.getStatus.mockResolvedValue(nativeStatus) computerUseApiMock.getAuthorizedApps.mockResolvedValue({ ...enabledConfig, @@ -391,7 +439,7 @@ describe('ComputerUseSettings', () => { render() - const toggle = await screen.findByLabelText('Any App') + const toggle = await screen.findByLabelText('Enabled') await waitFor(() => expect(toggle).not.toBeChecked()) await act(async () => { @@ -415,7 +463,7 @@ describe('ComputerUseSettings', () => { render() - const toggle = await screen.findByLabelText('Any App') + const toggle = await screen.findByLabelText('Enabled') await waitFor(() => expect(toggle).not.toBeChecked()) await act(async () => { diff --git a/desktop/src/pages/ComputerUseSettings.tsx b/desktop/src/pages/ComputerUseSettings.tsx index 593746b2..b2957f40 100644 --- a/desktop/src/pages/ComputerUseSettings.tsx +++ b/desktop/src/pages/ComputerUseSettings.tsx @@ -5,6 +5,7 @@ import { Button } from '@/components/ui/Button' import { ErrorState } from '@/components/ui/ErrorState' import { LoadingState } from '@/components/ui/LoadingState' import { Modal } from '@/components/ui/Modal' +import { Switch } from '@/components/ui/Switch' import { getDesktopHost } from '../lib/desktopHost' type CheckState = 'loading' | 'ready' | 'error' @@ -53,6 +54,7 @@ export function ComputerUseSettings() { const t = useTranslation() const [status, setStatus] = useState(null) const [checkState, setCheckState] = useState('loading') + const [configSettled, setConfigSettled] = useState(false) const [setupRunning, setSetupRunning] = useState(false) const [setupResult, setSetupResult] = useState(null) @@ -107,6 +109,8 @@ export function ComputerUseSettings() { applyConfig(await computerUseApi.getAuthorizedApps(), requestSeq) } catch { // API not ready + } finally { + setConfigSettled(true) } }, [applyConfig]) @@ -224,7 +228,7 @@ export function ComputerUseSettings() { } }, [t, fetchStatus]) - // Master "任意应用" toggle on the native path. Mirrors toggleComputerUseEnabled + // Master Computer Use toggle on the native path. Mirrors toggleComputerUseEnabled // for persistence, but additionally pops the native OS-permission card when // turning ON while macOS permissions are still missing (the headline flow). const toggleAnyApp = (value: boolean) => { @@ -370,6 +374,38 @@ export function ComputerUseSettings() { .sort((a, b) => a.displayName.localeCompare(b.displayName)) }, [filteredApps, authorizedBundleIds]) + // The renderer cannot choose between the native macOS page and the + // compatibility page until the capability probe finishes. Rendering the + // compatibility page here used to flash its header toggle before the native + // page replaced the entire tree. + if (status === null) { + return ( +
+ {checkState === 'error' ? ( + + ) : ( + + )} +
+ ) + } + + // Status chooses the page implementation, while config supplies the switch + // value. Waiting for both prevents a native disabled setting from briefly + // rendering as enabled when the capability probe wins the race. + if (!configSettled) { + return ( +
+ +
+ ) + } + if (native && status) { return ( {t('settings.computerUse.title')} - +

{t('settings.computerUse.description')} @@ -880,13 +913,21 @@ function NativeComputerUse({ return (

{/* Title */} -
-

- {t('settings.computerUse.controlTitle')} -

-

- {t('settings.computerUse.controlSubtitle')} -

+
+
+

+ {t('settings.computerUse.controlTitle')} +

+

+ {t('settings.computerUse.controlSubtitle')} +

+
+
{/* ─── 控制 (Control) ─── */} @@ -895,35 +936,11 @@ function NativeComputerUse({ {t('settings.computerUse.sectionControl')} - {/* One elevated surface; toggle + permissions split by a hairline rule - instead of two separately-nested filled cards. */} + {/* One elevated surface for the OS permissions required by the master + toggle in the page header. */}
- {/* Master "任意应用" toggle */} -
-
-
- {t('settings.computerUse.anyApp')} -
-
- {t('settings.computerUse.anyAppDesc')} -
-
- -
- {/* OS-permission group */} -
+
{t('settings.computerUse.osPermTitle')}
diff --git a/native/cu-helper/Sources/cu-helper/AXAction.swift b/native/cu-helper/Sources/cu-helper/AXAction.swift index ed7d8050..8b49fe1a 100644 --- a/native/cu-helper/Sources/cu-helper/AXAction.swift +++ b/native/cu-helper/Sources/cu-helper/AXAction.swift @@ -1182,12 +1182,12 @@ public enum AXAction { /// screenshot. It is gone. Driving an app while the user works in another /// one is the feature; an implementation that yanks their foreground to /// take a picture has traded away the thing being bought. When the raise is - /// not enough the router now fails the mutating action with - /// `window_occluded` instead of taking the screen. + /// not enough the router leaves the target covered. ScreenCaptureKit can + /// still capture that window independently, and PID/window-targeted input + /// remains valid; ordinary occlusion is not an input failure. /// - /// Called at most once per app per session (see the caller), because doing - /// it every time the user covers the window is arguing with them about - /// their own screen. + /// This is now only an explicit semantic action. State reads never call it + /// merely because another application covers the target. @discardableResult public static func raiseWindow(pid: pid_t, windowID: CGWindowID) -> Bool { let app = AXUIElementCreateApplication(pid) diff --git a/native/cu-helper/Sources/cu-helper/CommandRouter.swift b/native/cu-helper/Sources/cu-helper/CommandRouter.swift index 233861ef..b5258d83 100644 --- a/native/cu-helper/Sources/cu-helper/CommandRouter.swift +++ b/native/cu-helper/Sources/cu-helper/CommandRouter.swift @@ -99,7 +99,6 @@ public final class CommandRouter { resetHeldSessionState() AXTree.resetSessionSnapshots() Self.lastShotTransform.removeAll() - Self.recoveredTargets.removeAll() Self.lastCaptureDigest.removeAll() // Apps we told they were focused must be told they are not, or the // belief outlives the session that needed it. @@ -547,51 +546,6 @@ public final class CommandRouter { } } - /// Resolve the target app, build its key-window AX tree, and attach a - /// locked window screenshot (scale 0.5) when ScreenCaptureKit is available. - /// The screenshot is wrapped in `try?`: a capture failure (or SCK wedge) - /// must NEVER block the AX text — the text alone is a valid response. - /// Keep the target in a state where its own screenshot means something. - /// - /// Chromium stops producing frames for a fully covered window, so the - /// capture returns the last thing it painted — indefinitely, and without any - /// error. Recovered once per session per app: the user asked for this app to - /// be driven, and a window that cannot render cannot be driven. Burying it - /// again is them wanting their screen back, and we stop taking it. - /// - /// Returns the sentence to append to the state, or nil when all is well. - private func ensureTargetIsRenderable(pid: pid_t) -> String? { - guard let window = WindowGeometry.frontmostWindow(pid: pid) else { - // No on-screen window at all is a different problem, already - // reported by OffScreenTargetAdvice with its own advice. - return nil - } - guard WindowGeometry.isFullyCovered(windowID: window.id) else { return nil } - - switch TargetVisibilityPolicy.decide( - isFullyCovered: true, - hasRecoveredBefore: Self.recoveredTargets.contains(pid) - ) { - case .proceed: - return nil - case .warnOnly: - return TargetVisibilityPolicy.coveredAgainNotice - case .raiseAndNotify: - Self.recoveredTargets.insert(pid) - // A raise cannot get above another application's windows. Report - // which of the two happened rather than claiming the recovery - // worked — the notice is what the model uses to decide whether the - // screenshot can be trusted. - return AXAction.raiseWindow(pid: pid, windowID: window.id) - ? TargetVisibilityPolicy.raisedNotice - : TargetVisibilityPolicy.couldNotUncoverNotice - } - } - - /// Apps already pulled out from under other windows this session. Reset with - /// the rest of the session state so a new task starts willing to help again. - private static var recoveredTargets = Set() - /// Hash of the last capture per app, to notice when a new one is the same /// image. Hashes rather than the images themselves — these are megabytes. private static var lastCaptureDigest: [pid_t: Int] = [:] @@ -610,6 +564,14 @@ public final class CommandRouter { ) } + private static func appendAXNotice( + _ notice: String, + to object: inout [String: JSONValue] + ) { + let existing = object["axText"]?.asString ?? "" + object["axText"] = .string(existing + "\n\n" + notice) + } + private func handleGetAppState(_ payload: JSONValue) async throws -> JSONValue { let disableDiff = try optionalBoolean(payload, key: "disableDiff") ?? false let selector = try AppTargetResolver.requiredSelector(payload: payload) @@ -658,18 +620,8 @@ public final class CommandRouter { // target), not whatever happens to be frontmost. setResolvedTarget(target) - // Before photographing it: a window buried under everything else stops - // repainting, so the capture would freeze at whatever it last drew - // while every action still reported success. Recover it if we can, and - // in either case make sure the model is told. - let visibilityNotice = ensureTargetIsRenderable(pid: pid) - let result = try await AXTree.appState(pid: pid, disableDiff: disableDiff) var object = try encode(result).asObject ?? [:] - if let visibilityNotice { - let existing = object["axText"]?.asString ?? "" - object["axText"] = .string(existing + "\n\n" + visibilityNotice) - } guard let snapshotEvidence = AXTree.snapshotEvidence(pid: pid) else { throw CUError("stale_snapshot", "get_app_state did not publish target identity") } @@ -679,6 +631,12 @@ public final class CommandRouter { ) if let windowID = snapshotEvidence.keyWindowID { object["windowID"] = .int(Int(windowID)) + if WindowGeometry.isFullyCovered(windowID: windowID) { + Self.appendAXNotice( + TargetVisibilityPolicy.coveredCaptureNotice, + to: &object + ) + } } // A failed or mismatched fresh capture must not leave coordinates from @@ -710,8 +668,7 @@ public final class CommandRouter { base64: shot.base64, windowID: shot.windowID ) { - let existing = object["axText"]?.asString ?? "" - object["axText"] = .string(existing + "\n\n" + notice) + Self.appendAXNotice(notice, to: &object) } object["screenshot"] = .object([ "base64": .string(shot.base64), @@ -1415,10 +1372,6 @@ public final class CommandRouter { "The screen is locked, so the action was not run. Unlock the Mac and try again." ) } - // A fully covered Chromium/CEF window stops drawing and synthetic input - // cannot be verified. Failing closed is cheaper than reporting success - // for an action that may have gone nowhere. - try TargetVisibilityPolicy.ensureRenderableForMutation(pid: target.pid) let lease = try ForegroundLease.acquire( target: target, runtime: foregroundRuntime diff --git a/native/cu-helper/Sources/cu-helper/SyntheticWindowFocus.swift b/native/cu-helper/Sources/cu-helper/SyntheticWindowFocus.swift index 4eac93d4..de23f56b 100644 --- a/native/cu-helper/Sources/cu-helper/SyntheticWindowFocus.swift +++ b/native/cu-helper/Sources/cu-helper/SyntheticWindowFocus.swift @@ -35,12 +35,72 @@ import Foundation /// So this needs no private symbol at all — `NSEvent.otherEvent`, `.cgEvent` /// and `CGEventPostToPid` are public. Only the subtype values are undocumented. enum SyntheticWindowFocus { + struct BeliefTarget: Equatable, Sendable { + let processIdentity: AXTreeProcessIdentity? + } + + /// Session-scoped belief about process lifetimes that have already + /// received the synthetic focus + activation pair. + /// + /// Keeping this state is load-bearing for Chromium/CEF text entry. A click + /// establishes in-app focus on a field; blindly posting another + /// `keyFocusReturned` to window 0 before `type_text` can reset that field + /// focus. Codex keeps the same distinction between real application state + /// and what the target has already been told. + struct BeliefState { + private(set) var syntheticallyActive: [pid_t: BeliefTarget] = [:] + + /// Reserve the one establishment send for `pid`. + /// + /// Seeing the process genuinely active clears the synthetic belief so + /// a later background transition can establish it again. + mutating func beginEnforcement( + pid: pid_t, + applicationIsActive: Bool, + target: BeliefTarget + ) -> Bool { + if applicationIsActive { + observeRealActivation(pid: pid) + return false + } + guard syntheticallyActive[pid] != target else { return false } + syntheticallyActive[pid] = target + return true + } + + mutating func observeRealActivation(pid: pid_t) { + syntheticallyActive.removeValue(forKey: pid) + } + + mutating func cancelEnforcement( + pid: pid_t, + expectedTarget: BeliefTarget? = nil + ) { + if let expectedTarget, + syntheticallyActive[pid] != expectedTarget { + return + } + syntheticallyActive.removeValue(forKey: pid) + } + + mutating func drain() -> [pid_t: BeliefTarget] { + defer { syntheticallyActive.removeAll() } + return syntheticallyActive + } + } + + struct EnforcementRuntime: Sendable { + let applicationIsActive: Bool + let target: BeliefTarget + let post: @Sendable (Notification, pid_t) -> Bool + } + /// CPS notifications, carried as the subtype of a synthesized event. /// /// Values recovered from the once-initializers in Codex's service. Stored /// as `Int32` because `keyFocusReturned` does not fit `Int16` unsigned — /// see `subtype`. - enum Notification: Int32 { + enum Notification: Int32, Sendable { case appActivated = 1 // Also the value CPS uses for "new front process"; the meaning comes // from which notification the sender is making, not from the number. @@ -140,12 +200,80 @@ enum SyntheticWindowFocus { /// Order matches the reference: focus, then activation. @discardableResult static func enforceActiveState(pid: pid_t) -> Bool { - guard !isActiveApplication(pid) else { return false } - let focused = post(.keyFocusReturned, to: pid) - let activated = post(.appActivated, to: pid) - let posted = focused || activated - if posted { enforced.withLock { $0.insert(pid) } } - return posted + // Register before reserving belief so a real activation that happens + // later is observed even when no CU request runs while the app is + // actually frontmost. + _ = applicationLifecycleObserver + let runtime = EnforcementRuntime( + applicationIsActive: isActiveApplication(pid), + target: BeliefTarget( + processIdentity: currentProcessIdentity(pid: pid) + ), + post: { notification, targetPid in + post(notification, to: targetPid) + } + ) + let reserved = beliefs.withLock { state in + state.beginEnforcement( + pid: pid, + applicationIsActive: runtime.applicationIsActive, + target: runtime.target + ) + } + guard reserved else { return false } + guard postEnforcementPair(pid: pid, runtime: runtime) else { + beliefs.withLock { + $0.cancelEnforcement( + pid: pid, + expectedTarget: runtime.target + ) + } + return false + } + return true + } + + /// Testable transition used by the live wrapper above. Keeping the + /// notification sink beside the belief mutation lets tests drive the same + /// success, deduplication and rollback path production uses. + @discardableResult + static func enforceActiveState( + pid: pid_t, + state: inout BeliefState, + runtime: EnforcementRuntime + ) -> Bool { + guard state.beginEnforcement( + pid: pid, + applicationIsActive: runtime.applicationIsActive, + target: runtime.target + ) else { return false } + + guard postEnforcementPair(pid: pid, runtime: runtime) else { + state.cancelEnforcement(pid: pid, expectedTarget: runtime.target) + return false + } + return true + } + + /// Post outside the belief lock. AppKit event construction and delivery + /// are external calls; keeping an unfair lock held across them risks + /// re-entrancy and makes every other focus transition wait unnecessarily. + private static func postEnforcementPair( + pid: pid_t, + runtime: EnforcementRuntime + ) -> Bool { + let focused = runtime.post(.keyFocusReturned, pid) + let activated = runtime.post(.appActivated, pid) + guard focused && activated else { + if focused || activated { + // Do not leave a half-established belief behind when AppKit + // could construct only one side of the pair. + if focused { _ = runtime.post(.lostKeyFocus, pid) } + _ = runtime.post(.appDeactivated, pid) + } + return false + } + return true } /// Reality, as opposed to what the target has been told. @@ -153,6 +281,51 @@ enum SyntheticWindowFocus { NSWorkspace.shared.frontmostApplication?.processIdentifier == pid } + private static func currentProcessIdentity(pid: pid_t) -> AXTreeProcessIdentity? { + guard let application = NSRunningApplication(processIdentifier: pid) else { + return nil + } + return AXTreeProcessIdentity( + bundleID: application.bundleIdentifier, + executablePath: application.executableURL?.path, + launchTime: application.launchDate?.timeIntervalSinceReferenceDate + ) + } + + private static func observeRealActivation(pid: pid_t) { + beliefs.withLock { $0.observeRealActivation(pid: pid) } + } + + /// NSWorkspace is the observable edge the old PID-only cache lacked. If a + /// user brings a synthetic target to the real foreground and then leaves + /// it, the real deactivate invalidates what the app was told. Clearing the + /// belief on activation makes the next background request establish a new + /// pair instead of trusting stale session state. + private final class ApplicationLifecycleObserver: @unchecked Sendable { + private let activationToken: NSObjectProtocol + + init() { + activationToken = NSWorkspace.shared.notificationCenter.addObserver( + forName: NSWorkspace.didActivateApplicationNotification, + object: nil, + queue: .main + ) { notification in + let application = notification.userInfo?[NSWorkspace.applicationUserInfoKey] + as? NSRunningApplication + guard let pid = application?.processIdentifier, + NSWorkspace.shared.frontmostApplication?.processIdentifier == pid + else { return } + SyntheticWindowFocus.observeRealActivation(pid: pid) + } + } + + deinit { + NSWorkspace.shared.notificationCenter.removeObserver(activationToken) + } + } + + private static let applicationLifecycleObserver = ApplicationLifecycleObserver() + /// Tell every app we lied to that it is no longer active. /// /// Scoped to the session, not the action: re-sending per click would cancel @@ -165,14 +338,20 @@ enum SyntheticWindowFocus { /// an app the user is not in, and the next real click there arrives at a /// window that never learned it had lost focus. static func relinquishAll() { - let pids = enforced.withLock { pids -> Set in - defer { pids.removeAll() } - return pids + let targets = beliefs.withLock { $0.drain() } + for (pid, target) in targets { + // Do not aim teardown at a recycled PID, or tell an app the user is + // genuinely using that it lost focus. + guard !isActiveApplication(pid), + currentProcessIdentity(pid: pid) == target.processIdentity + else { continue } + post(.lostKeyFocus, to: pid) + post(.appDeactivated, to: pid) } - for pid in pids { post(.appDeactivated, to: pid) } } - /// Targets currently told they are focused. Written from the daemon's - /// request queue and read on teardown, so it needs the lock. - private static let enforced = OSAllocatedUnfairLock(initialState: Set()) + /// Written from the daemon's request queue and read on teardown. The lock + /// also makes the reserve-before-send transition atomic if a future caller + /// reaches it off the main actor. + private static let beliefs = OSAllocatedUnfairLock(initialState: BeliefState()) } diff --git a/native/cu-helper/Sources/cu-helper/TargetVisibilityPolicy.swift b/native/cu-helper/Sources/cu-helper/TargetVisibilityPolicy.swift index edd4632a..78929185 100644 --- a/native/cu-helper/Sources/cu-helper/TargetVisibilityPolicy.swift +++ b/native/cu-helper/Sources/cu-helper/TargetVisibilityPolicy.swift @@ -1,159 +1,43 @@ -import CoreGraphics import Foundation -/// Decides what to do when the app being driven is buried under other windows. +/// Explains a repeated window capture without turning ordinary occlusion into +/// an input failure. /// -/// A fully covered Chromium window stops drawing, so the screenshot freezes -/// while every action still reports success. That combination is worse than an -/// error: on a real session the model went three and a half minutes on one -/// stale image and then reported a song playing that the final capture plainly -/// showed paused. The state has to be recoverable, and when it is not, said out -/// loud. -/// -/// The judgement this encodes is where to stop. Raising the window once is -/// helping — the user asked for this app to be driven, and a window that cannot -/// render cannot be driven. Raising it every time it gets covered is fighting -/// the user for their own screen, and the second burial is the signal that they -/// meant it. -/// -/// What is NOT on the table is activating the application. A raise reorders a -/// window; activation takes the user's foreground, and driving an app while -/// they work in another one is the entire feature. When a raise is not enough -/// to uncover the window, this says so and stops — a stalled turn is a cheaper -/// price than the user's screen. See `AXAction.raiseWindow`. +/// ScreenCaptureKit captures a target window independently of the desktop's +/// stacking order, and all fallback input is addressed to the target process +/// and window. Another app covering the target is therefore not a reason to +/// raise it, activate it, or reject a mutation. A byte-identical capture is +/// still useful evidence, but only about pixels: it cannot prove that an action +/// failed, and it must never stop background automation. enum TargetVisibilityPolicy { - enum Action: Equatable { - /// Visible enough to keep rendering. Say nothing. - case proceed - /// Bring it back and tell the model what happened, so a screenshot that - /// changes for no apparent reason is explained. - case raiseAndNotify - /// Covered again after we already recovered it once. Leave the user's - /// windows alone and downgrade the screenshot's credibility instead. - case warnOnly - } - - /// - Parameters: - /// - isFullyCovered: from `WindowGeometry.isFullyCovered`. - /// - hasRecoveredBefore: whether this session already raised this target. - static func decide( - isFullyCovered: Bool, - hasRecoveredBefore: Bool - ) -> Action { - guard isFullyCovered else { return .proceed } - return hasRecoveredBefore ? .warnOnly : .raiseAndNotify - } - - /// Fail closed when the target window is fully covered. - /// - /// A fully covered Chromium/CEF window stops drawing, and synthetic input - /// delivered to it cannot be verified. Continuing to report "Action - /// completed" in that state is worse than an error, because the caller has - /// no way to know whether the action landed. This guard is called before - /// every mutating command. - static func ensureRenderableForMutation( - pid: pid_t, - frontmostWindow: (pid_t) -> WindowGeometry.Window? = { WindowGeometry.frontmostWindow(pid: $0) }, - isFullyCovered: (CGWindowID) -> Bool = { WindowGeometry.isFullyCovered(windowID: $0) } - ) throws { - guard let window = frontmostWindow(pid) else { return } - guard isFullyCovered(window.id) else { return } - throw CUError( - "window_occluded", - """ - The target app's window is completely covered by other windows, so the \ - action was not sent. Input cannot reliably reach an occluded Chromium/CEF \ - window. Uncover the window and try again. - """ - ) - } - - static let raisedNotice = """ - NOTE: This app's window was completely covered by other windows, which stops \ - it redrawing — the screenshot would have been frozen at whatever it last \ - painted. It has been brought back to the front once so the state you are \ - shown is live. Expect the picture to look different from the previous turn \ - for that reason alone. + static let coveredCaptureNotice = """ + NOTE: Another application fully covers the target window. The screenshot \ + is captured from that window rather than from the visible desktop, but it \ + may be stale if the target paused its renderer while covered. Coverage does \ + not block Accessibility actions or app- and window-targeted input; continue \ + the task without activating or raising the target just to expose it. """ - static let coveredAgainNotice = """ - NOTE: This app's window is completely covered by other windows again. It was \ - already restored once this session, so it has been left alone — putting it \ - back a second time would be taking the screen from the user, who evidently \ - wants it covered. - - \(staleCaptureWarning) - """ - - /// Said when the raise ran and the window is still buried. - /// - /// The honest report of a limit, not a failure to try. Only activating the - /// application would clear this, and that takes the user's foreground — - /// which is the one thing the feature exists to avoid. - static let couldNotUncoverNotice = """ - NOTE: This app's window is completely covered by other windows. Raising the \ - window did not clear it, and it has been left there: getting above another \ - application's windows would mean activating this app and taking the \ - foreground away from whatever the user is doing. - - \(staleCaptureWarning) - """ - - /// Shared by both covered cases: the screenshot is stale and mutating - /// actions are refused. - /// - /// The first version of this ended with "ask the user to leave it visible", - /// and that is what the model did — it stopped one action into a three-step - /// task and handed the job back. A user who asks for automation is not - /// asking to be consulted about window management halfway through. - /// - /// The failure this file was written for is the opposite one: a session that - /// read a frozen image, pressed play four times, and reported a song playing - /// that the capture showed paused. Both are avoidable at once, because the - /// engine now fails closed: mutating actions are not attempted while the - /// window is fully covered. The model can still read the AX tree and report - /// what it sees, but it cannot change state through an occluded window. - private static let staleCaptureWarning = """ - Treat the screenshot as possibly STALE: a covered window stops redrawing, so \ - it may show what the app last painted rather than what is true now. - - Mutating actions are refused while the window stays fully covered, because \ - input cannot reliably reach an occluded Chromium/CEF window and its effect \ - cannot be verified. Uncover the window to continue. - """ - - /// Said when a mutating action produced a byte-identical capture. - /// - /// Which sentence depends on coverage, because we know the answer and the - /// model does not. The first draft offered both possibilities — "the action - /// missed, or the window is not repainting, you cannot tell" — while the - /// window was demonstrably repainting the whole time. The model spent four - /// minutes chasing the explanation we had handed it and never revisited the - /// true one. An engine that can check a thing must not offer it as a - /// mystery. - /// - /// - Parameter windowIsCovered: whether the target is fully buried, i.e. - /// whether "it stopped repainting" is actually on the table. static func identicalCaptureNotice(windowIsCovered: Bool) -> String { let cause = windowIsCovered ? """ - The window is also fully covered right now, so it may have stopped \ - repainting: this image can be older than it looks. Mutating actions \ - are refused while the window is covered, so the picture will stay \ - frozen until it is uncovered. + The target window is covered, so this image may be older than it \ + looks if that app paused its renderer. Coverage does not block \ + app- and window-targeted input. """ : """ - The window is visible and repainting, so this is not a stale \ - image — the action genuinely changed nothing. + The target window is not fully covered, so coverage does not \ + explain the identical pixels, and no visible pixel change was \ + observed. """ - return """ - NOTE: This screenshot is byte-for-byte identical to the previous one, \ - taken after an action that should have changed something. \(cause) - Do not repeat the same action. If it is a toggle such as play/pause, a \ - second press undoes the first — one session pressed play four times on \ - unchanging pictures and finished paused. Continue the task, and report \ - what you did and what you could not confirm. + return """ + NOTE: This screenshot is byte-for-byte identical to the previous one. \ + \(cause) + + Do not blindly repeat the same toggle: a second play/pause press can \ + undo the first. Re-read the accessibility state, use a semantic action \ + when one is available, or continue with a different app-targeted step. """ } } diff --git a/native/cu-helper/Tests/CuHelperTests/SyntheticWindowFocusTests.swift b/native/cu-helper/Tests/CuHelperTests/SyntheticWindowFocusTests.swift index 9e0f72c1..7452ed5b 100644 --- a/native/cu-helper/Tests/CuHelperTests/SyntheticWindowFocusTests.swift +++ b/native/cu-helper/Tests/CuHelperTests/SyntheticWindowFocusTests.swift @@ -1,5 +1,6 @@ import AppKit import CoreGraphics +import os import XCTest @testable import cc_haha_computer_use @@ -13,6 +14,20 @@ import XCTest /// real app. What is worth pinning is the one value that is undocumented, has /// no error path, and silently means nothing if it is wrong. final class SyntheticWindowFocusTests: XCTestCase { + private let processA = AXTreeProcessIdentity( + bundleID: "com.example.target", + executablePath: "/Applications/Target.app/Contents/MacOS/Target", + launchTime: 1 + ) + + private func beliefTarget( + processIdentity: AXTreeProcessIdentity? = nil + ) -> SyntheticWindowFocus.BeliefTarget { + SyntheticWindowFocus.BeliefTarget( + processIdentity: processIdentity ?? processA + ) + } + func testKeyFocusReturnedSurvivesTheSignedSubtypeField() { // `NSEvent.subtype` is Int16. 0x8000 does not fit, and the obvious // conversions either trap or clamp to 0x7FFF; truncating to the same @@ -89,9 +104,180 @@ final class SyntheticWindowFocusTests: XCTestCase { XCTAssertNotNil(event.cgEvent, "must survive conversion or it cannot be posted") } - /// Establishing focus takes two notifications, and sending one was half the - /// reason background actuation never worked. - func testEnforcingActiveStateSendsBothBeliefs() throws { + func testAnInvalidPidIsRefusedRatherThanBroadcast() { + // CGEventPostToPid with a nonsense pid is not obviously harmless, and a + // focus notification aimed at nothing is never something we meant. + XCTAssertFalse(SyntheticWindowFocus.post(.keyFocusReturned, to: 0)) + XCTAssertFalse(SyntheticWindowFocus.post(.keyFocusReturned, to: -1)) + } + + func testEnforcementPostsOneCompletePairForTheSameTarget() { + var state = SyntheticWindowFocus.BeliefState() + let target = beliefTarget() + let sent = OSAllocatedUnfairLock( + initialState: [SyntheticWindowFocus.Notification]() + ) + let runtime = SyntheticWindowFocus.EnforcementRuntime( + applicationIsActive: false, + target: target, + post: { notification, _ in + sent.withLock { $0.append(notification) } + return true + } + ) + + XCTAssertTrue(SyntheticWindowFocus.enforceActiveState( + pid: 42, + state: &state, + runtime: runtime + )) + XCTAssertEqual(sent.withLock { $0 }, [.keyFocusReturned, .appActivated]) + + // click -> type_text is one focus transaction. Re-establishing focus + // between those actions can reset the CEF control the click selected. + XCTAssertFalse(SyntheticWindowFocus.enforceActiveState( + pid: 42, + state: &state, + runtime: runtime + )) + XCTAssertEqual(sent.withLock { $0 }, [.keyFocusReturned, .appActivated]) + XCTAssertEqual(state.syntheticallyActive, [42: target]) + } + + func testPartialEnforcementIsWithdrawnAndCanRetry() { + struct PostingState: Sendable { + var sent: [SyntheticWindowFocus.Notification] = [] + var failActivation = true + } + + var state = SyntheticWindowFocus.BeliefState() + let target = beliefTarget() + let posting = OSAllocatedUnfairLock(initialState: PostingState()) + let runtime = SyntheticWindowFocus.EnforcementRuntime( + applicationIsActive: false, + target: target, + post: { notification, _ in + posting.withLock { state in + state.sent.append(notification) + if notification == .appActivated, state.failActivation { + state.failActivation = false + return false + } + return true + } + } + ) + + XCTAssertFalse(SyntheticWindowFocus.enforceActiveState( + pid: 42, + state: &state, + runtime: runtime + )) + XCTAssertEqual( + posting.withLock { $0.sent }, + [.keyFocusReturned, .appActivated, .lostKeyFocus, .appDeactivated] + ) + XCTAssertTrue(state.syntheticallyActive.isEmpty) + + posting.withLock { $0.sent.removeAll() } + XCTAssertTrue(SyntheticWindowFocus.enforceActiveState( + pid: 42, + state: &state, + runtime: runtime + )) + XCTAssertEqual( + posting.withLock { $0.sent }, + [.keyFocusReturned, .appActivated] + ) + XCTAssertEqual(state.syntheticallyActive, [42: target]) + } + + /// A click followed by type_text is one focus transaction, not two. + /// Re-sending keyFocusReturned to window 0 between them can clear the CEF + /// field the click just focused. + func testSyntheticBeliefIsEstablishedOnlyOnceUntilReleased() { + var state = SyntheticWindowFocus.BeliefState() + let target = beliefTarget() + + XCTAssertTrue(state.beginEnforcement( + pid: 42, + applicationIsActive: false, + target: target + )) + XCTAssertFalse(state.beginEnforcement( + pid: 42, + applicationIsActive: false, + target: target + )) + XCTAssertEqual(state.syntheticallyActive, [42: target]) + + XCTAssertEqual(state.drain(), [42: target]) + XCTAssertTrue(state.syntheticallyActive.isEmpty) + XCTAssertTrue(state.beginEnforcement( + pid: 42, + applicationIsActive: false, + target: target + )) + } + + func testRealActivationSupersedesSyntheticBelief() { + var state = SyntheticWindowFocus.BeliefState() + let target = beliefTarget() + + XCTAssertTrue(state.beginEnforcement( + pid: 42, + applicationIsActive: false, + target: target + )) + state.observeRealActivation(pid: 42) + XCTAssertTrue(state.syntheticallyActive.isEmpty) + + // Once the app is background again, it needs a fresh pair. + XCTAssertTrue(state.beginEnforcement( + pid: 42, + applicationIsActive: false, + target: target + )) + XCTAssertFalse(state.beginEnforcement( + pid: 42, + applicationIsActive: true, + target: target + )) + state.cancelEnforcement(pid: 42) + XCTAssertTrue(state.syntheticallyActive.isEmpty) + } + + func testOnlyAProcessLifetimeChangeRequiresFreshBelief() { + var state = SyntheticWindowFocus.BeliefState() + let originalProcess = beliefTarget() + let relaunchedProcess = AXTreeProcessIdentity( + bundleID: processA.bundleID, + executablePath: processA.executablePath, + launchTime: 2 + ) + let relaunched = beliefTarget( + processIdentity: relaunchedProcess + ) + + XCTAssertTrue(state.beginEnforcement( + pid: 42, + applicationIsActive: false, + target: originalProcess + )) + XCTAssertFalse(state.beginEnforcement( + pid: 42, + applicationIsActive: false, + target: originalProcess + )) + XCTAssertTrue(state.beginEnforcement( + pid: 42, + applicationIsActive: false, + target: relaunched + )) + XCTAssertEqual(state.syntheticallyActive[42], relaunched) + } + + func testTeardownWithdrawsFocusBeforeActivationBelief() throws { let source = try String( contentsOfFile: URL(fileURLWithPath: #filePath) .deletingLastPathComponent() @@ -102,26 +288,13 @@ final class SyntheticWindowFocusTests: XCTestCase { encoding: .utf8 ) let body = try XCTUnwrap( - source.range(of: "static func enforceActiveState").map { - String(source[$0.lowerBound...].prefix(600)) + source.range(of: "static func relinquishAll").map { + String(source[$0.lowerBound...].prefix(1_000)) } ) - XCTAssertTrue( - body.contains("post(.keyFocusReturned"), - "the target must be told its window has focus" - ) - XCTAssertTrue( - body.contains("post(.appActivated"), - "the target must also be told its application is active — " - + "focus alone leaves input routing where it was" - ) - } - - func testAnInvalidPidIsRefusedRatherThanBroadcast() { - // CGEventPostToPid with a nonsense pid is not obviously harmless, and a - // focus notification aimed at nothing is never something we meant. - XCTAssertFalse(SyntheticWindowFocus.post(.keyFocusReturned, to: 0)) - XCTAssertFalse(SyntheticWindowFocus.post(.keyFocusReturned, to: -1)) + let lostFocus = try XCTUnwrap(body.range(of: "post(.lostKeyFocus")) + let deactivated = try XCTUnwrap(body.range(of: "post(.appDeactivated")) + XCTAssertLessThan(lostFocus.lowerBound, deactivated.lowerBound) } func testItNeedsNoPrivateSymbols() throws { @@ -256,15 +429,45 @@ final class InputAcceptanceContractTests: XCTestCase { // documented that gate and shipped without it — sending "key focus // returned to window 0" to an app that already owned a key window, on // every click. - let focus = try source("SyntheticWindowFocus.swift") - let enforce = try XCTUnwrap( - focus.range(of: "static func enforceActiveState").map { - String(focus[$0.lowerBound...].prefix(400)) + var state = SyntheticWindowFocus.BeliefState() + let sent = OSAllocatedUnfairLock( + initialState: [SyntheticWindowFocus.Notification]() + ) + let activeRuntime = SyntheticWindowFocus.EnforcementRuntime( + applicationIsActive: true, + target: SyntheticWindowFocus.BeliefTarget(processIdentity: nil), + post: { notification, _ in + sent.withLock { $0.append(notification) } + return true } ) - XCTAssertTrue( - enforce.contains("isActiveApplication"), - "must not tell an already-active app that focus returned" + + XCTAssertFalse(SyntheticWindowFocus.enforceActiveState( + pid: 42, + state: &state, + runtime: activeRuntime + )) + XCTAssertTrue(sent.withLock { $0 }.isEmpty) + XCTAssertTrue(state.syntheticallyActive.isEmpty) + + let backgroundRuntime = SyntheticWindowFocus.EnforcementRuntime( + applicationIsActive: false, + target: activeRuntime.target, + post: activeRuntime.post + ) + XCTAssertTrue(SyntheticWindowFocus.enforceActiveState( + pid: 42, + state: &state, + runtime: backgroundRuntime + )) + XCTAssertFalse(SyntheticWindowFocus.enforceActiveState( + pid: 42, + state: &state, + runtime: backgroundRuntime + )) + XCTAssertEqual( + sent.withLock { $0 }, + [.keyFocusReturned, .appActivated] ) } } diff --git a/native/cu-helper/Tests/CuHelperTests/TargetVisibilityTests.swift b/native/cu-helper/Tests/CuHelperTests/TargetVisibilityTests.swift index 25af68dd..c3cdcbf5 100644 --- a/native/cu-helper/Tests/CuHelperTests/TargetVisibilityTests.swift +++ b/native/cu-helper/Tests/CuHelperTests/TargetVisibilityTests.swift @@ -64,143 +64,68 @@ final class WindowCoverageTests: XCTestCase { } final class TargetVisibilityPolicyTests: XCTestCase { - func testAVisibleTargetIsLeftAlone() { - XCTAssertEqual( - TargetVisibilityPolicy.decide(isFullyCovered: false, hasRecoveredBefore: false), - .proceed - ) - // Even after a recovery, visible means silent: a notice on every - // subsequent turn would be noise the model learns to skip. - XCTAssertEqual( - TargetVisibilityPolicy.decide(isFullyCovered: false, hasRecoveredBefore: true), - .proceed - ) + private func source(_ name: String) throws -> String { + let root = URL(fileURLWithPath: #filePath) + .deletingLastPathComponent() + .deletingLastPathComponent() + .deletingLastPathComponent() + .appendingPathComponent("Sources/cu-helper") + return try String(contentsOf: root.appendingPathComponent(name), encoding: .utf8) } - func testTheFirstBurialIsRecovered() { - // A window that cannot render cannot be driven, and the user asked for - // it to be driven. Raising it once is honouring the request. - XCTAssertEqual( - TargetVisibilityPolicy.decide(isFullyCovered: true, hasRecoveredBefore: false), - .raiseAndNotify - ) - } - - func testTheSecondBurialIsNotFought() { - // The whole judgement of this file: covering it again is the user - // saying they want their screen. Taking it back would be the automation - // arguing with them, once per turn, for as long as the task runs. - XCTAssertEqual( - TargetVisibilityPolicy.decide(isFullyCovered: true, hasRecoveredBefore: true), - .warnOnly - ) - } - - func testTheWarningRemovesTheScreenshotsAuthorityRatherThanJustDescribingIt() { - // "The window is covered" alone leaves the model free to keep reading - // the picture — which is what produced a confident report of a song - // playing that the capture showed paused. The notice has to say the - // image cannot settle the question, and that mutating actions are refused. - // - // Both covered outcomes carry it. `couldNotUncoverNotice` is the newer - // one and the easier to get wrong: it reads as "we tried and it is - // fine", when the screenshot behind it is exactly as untrustworthy. - for notice in [ - TargetVisibilityPolicy.coveredAgainNotice, - TargetVisibilityPolicy.couldNotUncoverNotice, - ] { - XCTAssertTrue(notice.contains("STALE")) - XCTAssertTrue( - notice.contains("Mutating actions are refused"), - "a covered window must not invite the model to act blindly" - ) - XCTAssertTrue( - notice.contains("cannot reliably reach"), - "the reason for refusal must be stated" - ) - } - } - - /// A covered window must not end the task by asking the user to manage it. + /// Regression for session ba87fe5e-a2dc-4d93-932f-4c868df4c05e. /// - /// The first version of these notices closed with "ask the user to leave it - /// visible", and the model obeyed: it stopped after one action of a - /// three-step instruction and handed the job back. Stopping to consult the - /// user about window management is the automation failing, and it is a very - /// easy sentence to reintroduce while tightening the honesty language — - /// which is the half of this that keeps pulling the other way. - /// - /// The current contract is different: the engine fails closed, so the - /// notice explains *why* actions are refused and tells the user to uncover - /// the window. It must not ask the model to ask the user. - func testACoveredWindowDoesNotTurnIntoAQuestionForTheUser() { - for notice in [ - TargetVisibilityPolicy.coveredAgainNotice, - TargetVisibilityPolicy.couldNotUncoverNotice, - ] { - XCTAssertTrue( - notice.contains("Uncover the window to continue"), - "the user needs a concrete next step, not a stalled turn" - ) - // The exact shape of the sentence that caused it: an instruction to - // put the question to the user. Kept as a literal because the - // failure was literal. - XCTAssertFalse( - notice.lowercased().contains("ask the user to"), - "handing window management back to the user abandons the task" - ) - } + /// `withForegroundLease` used to reject every mutation when another app + /// covered the target. That put the guard in front of PID/window-targeted + /// input and even in front of the AX `Raise` action the model tried as a + /// recovery, so the task had no possible next move. + func testOcclusionIsNotAMutationGate() throws { + let router = try source("CommandRouter.swift") + let body = try XCTUnwrap( + router.range(of: "private func withForegroundLease").map { + String(router[$0.lowerBound...]) + }, + "withForegroundLease is missing" + ) + XCTAssertFalse(body.contains("ensureRenderableForMutation")) + XCTAssertFalse(body.contains("window_occluded")) + + // Coverage is still measured for capture diagnostics elsewhere in the + // router. Pin the absence of the old *gate* across the whole file so a + // longer function cannot silently outrun a prefix-based source test. + XCTAssertFalse(router.contains("ensureRenderableForMutation")) + XCTAssertFalse(router.contains("window_occluded")) } - /// Honesty now means: the picture is stale and mutating actions are refused. - /// - /// The old notice claimed input did not depend on visibility; measured - /// behavior under occlusion showed it does for Chromium/CEF windows. The - /// replacement must not revive that claim. - func testTheModelIsToldActionsAreRefusedWhileCovered() { - for notice in [ - TargetVisibilityPolicy.coveredAgainNotice, - TargetVisibilityPolicy.couldNotUncoverNotice, - ] { - XCTAssertTrue(notice.contains("Mutating actions are refused")) - XCTAssertFalse( - notice.contains("Input does not depend on visibility"), - "old claim contradicted by measured occlusion behavior" - ) - XCTAssertFalse( - notice.contains("Carry on with the task"), - "the engine now fails closed, so the model must not be told to proceed" - ) - } + /// Reading state must not reorder or activate a target just because the + /// user covered it. Window-independent capture and app-targeted input are + /// what make this background automation rather than foreground automation. + func testGetAppStateDoesNotRaiseAnOccludedTarget() throws { + let router = try source("CommandRouter.swift") + let body = try XCTUnwrap( + router.range(of: "private func handleGetAppState").flatMap { start in + router.range(of: "struct ShotTransform", range: start.upperBound..