diff --git a/.github/workflows/macos-ci.yml b/.github/workflows/macos-ci.yml index 2fd69da9a..f7436efb5 100644 --- a/.github/workflows/macos-ci.yml +++ b/.github/workflows/macos-ci.yml @@ -107,11 +107,18 @@ jobs: - name: Show Swift toolchain version run: swift --version - - name: Run mutation inventory and selector contracts + - name: Run mutation inventory, selector, and host window contracts working-directory: Core/PeekabooAutomationKit + env: + RUN_AUTOMATION_ACTIONS: "true" run: | + set -o pipefail + test_log="$RUNNER_TEMP/host-window-contracts.log" swift test --no-parallel \ - --filter 'SystemIdentityResolverTests|ApplicationInventoryTimeoutTests|ApplicationMutationPlannerTests|ApplicationMutationSelectorTests|ApplicationIdentifierMatcherTests' + --filter 'SystemIdentityResolverTests|ApplicationInventoryTimeoutTests|ApplicationMutationPlannerTests|ApplicationMutationSelectorTests|ApplicationIdentifierMatcherTests|SameProcessWindowCloseTests' \ + 2>&1 | tee "$test_log" + grep -Fq 'Suite SameProcessWindowCloseTests passed after ' "$test_log" + grep -Eq 'Test run with [1-9][0-9]* tests?( in [0-9]+ suites?)? passed after ' "$test_log" - name: Build PeekabooCore working-directory: Core/PeekabooCore diff --git a/CHANGELOG.md b/CHANGELOG.md index 666a5b023..bbbb85c22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Keep background window close, restore, and maximize callbacks on the main thread when the target belongs to the Peekaboo host, preventing embedded macOS apps from crashing while preserving exact-window validation and remote AX deadlines. + - Keep caller screenshot destinations intact when remote evidence is rejected or raw output was not requested, staging ordinary captures before file publication as well as ROI captures. #710. - Fix host-routed screen observations with Accessibility elements by validating their semantic owner separately from the screen raster target. #715, #710. diff --git a/Core/PeekabooAutomationKit/Package.swift b/Core/PeekabooAutomationKit/Package.swift index 0570553c4..b400b95a9 100644 --- a/Core/PeekabooAutomationKit/Package.swift +++ b/Core/PeekabooAutomationKit/Package.swift @@ -56,12 +56,18 @@ let package = Package( name: "DescriptorForkTestSupport", path: "Tests/Support/DescriptorForkTestSupport", publicHeadersPath: "include"), + .executableTarget( + name: "HostWindowCloseFixture", + dependencies: ["PeekabooAutomationKit"], + path: "Tests/Support/HostWindowCloseFixture", + swiftSettings: approachableConcurrencySettings), .testTarget( name: "PeekabooAutomationKitTests", dependencies: [ "PeekabooAutomationKit", "PeekabooAutomationKitTestSupport", "DescriptorForkTestSupport", + "HostWindowCloseFixture", .product(name: "PeekabooFoundationTestSupport", package: "PeekabooFoundation"), ], path: "Tests/PeekabooAutomationKitTests", diff --git a/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/AXChildWindowMessagingTimeout.swift b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/AXChildWindowMessagingTimeout.swift index 60e8432ad..6ebc63535 100644 --- a/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/AXChildWindowMessagingTimeout.swift +++ b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/AXChildWindowMessagingTimeout.swift @@ -1,7 +1,7 @@ import ApplicationServices -/// Applies and clears an unchecked per-element AX deadline for detached workers. -/// Raw workers run off MainActor and cannot use AXorcist's checked `Element.withMessagingTimeout` scope. +/// Applies and clears an unchecked per-element AX deadline for raw AX operations. +/// Detached workers cannot use AXorcist's MainActor-checked `Element.withMessagingTimeout` scope. /// Application deadlines do not cover returned child references; each child needs its own scope. enum AXChildWindowMessagingTimeout { static func perform( diff --git a/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/WindowManagementService+StateOperations.swift b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/WindowManagementService+StateOperations.swift index aa6779161..f4115f84e 100644 --- a/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/WindowManagementService+StateOperations.swift +++ b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/WindowManagementService+StateOperations.swift @@ -1084,14 +1084,13 @@ extension CGPoint { } } -/// Runs the only blocking AX calls used by background close/maximize away from MainActor and -/// applies a per-element messaging deadline. Cancellation of the caller cannot stop a synchronous -/// Accessibility message already in the kernel, so the native AX deadline is the hard safety bound. +/// Keeps remote AX messaging off MainActor with per-element deadlines. In-process AX calls +/// synchronously invoke AppKit, so host-owned windows must stay on MainActor. private enum BoundedBackgroundWindowAX { private static let messagingTimeout: Float = 0.75 static func windowPresence(expectedIdentity: WindowMutationIdentity) async -> PinnedMinimizedWindowAXPresence { - await Task.detached(priority: .userInitiated) { + await self.perform(expectedIdentity: expectedIdentity) { let processStartIdentityBeforeScan = SystemIdentityResolver.processStartIdentity( expectedIdentity.ownerProcessIdentifier) guard SystemIdentityResolver.validateWindowMutationOwnerGeneration(expectedIdentity), @@ -1109,11 +1108,11 @@ private enum BoundedBackgroundWindowAX { processStartIdentityBeforeScan: processStartIdentityBeforeScan, processStartIdentityAfterScan: processStartIdentityAfterScan, scan: scan) - }.value + } } static func dispatchMinimizedRestore(expectedIdentity: WindowMutationIdentity) async -> Bool { - await Task.detached(priority: .userInitiated) { + await self.perform(expectedIdentity: expectedIdentity) { guard expectedIdentity.isMinimized == true, let windowID = CGWindowID(exactly: expectedIdentity.windowID), SystemIdentityResolver.validateWindowMutationOwnerGeneration(expectedIdentity), @@ -1151,14 +1150,14 @@ private enum BoundedBackgroundWindowAX { kAXMinimizedAttribute as CFString, kCFBooleanFalse) == .success } - }.value + } } static func dispatchClose( expectedIdentity: WindowMutationIdentity, action: BoundedBackgroundWindowCloseAction) async -> Bool { - await Task.detached(priority: .userInitiated) { + await self.perform(expectedIdentity: expectedIdentity) { guard SystemIdentityResolver.validateWindowMutationIdentity(expectedIdentity), let capturedBounds = expectedIdentity.capturedBounds, let windowID = CGWindowID(exactly: expectedIdentity.windowID), @@ -1213,14 +1212,14 @@ private enum BoundedBackgroundWindowAX { } } } - }.value + } } static func setBounds( expectedIdentity: WindowMutationIdentity, bounds: CGRect) async -> BackgroundWindowGeometryDispatchResult { - await Task.detached(priority: .userInitiated) { + await self.perform(expectedIdentity: expectedIdentity) { guard SystemIdentityResolver.validateWindowMutationIdentity(expectedIdentity), let capturedBounds = expectedIdentity.capturedBounds, let windowID = CGWindowID(exactly: expectedIdentity.windowID), @@ -1272,7 +1271,17 @@ private enum BoundedBackgroundWindowAX { identityRemainedPinned: liveProcessStartIdentity == expectedIdentity.ownerProcessStartIdentity && resolvedCandidateWindowID == expectedIdentity.windowID) } - }.value + } + } + + private static func perform( + expectedIdentity: WindowMutationIdentity, + operation: @escaping @Sendable () -> Result) async -> Result + { + if expectedIdentity.ownerProcessIdentifier == ProcessInfo.processInfo.processIdentifier { + return await MainActor.run(body: operation) + } + return await Task.detached(priority: .userInitiated, operation: operation).value } private static func exactWindow(windowID: CGWindowID, ownerPID: pid_t) -> AXUIElement? { diff --git a/Core/PeekabooAutomationKit/Tests/PeekabooAutomationKitTests/SameProcessWindowCloseTests.swift b/Core/PeekabooAutomationKit/Tests/PeekabooAutomationKitTests/SameProcessWindowCloseTests.swift new file mode 100644 index 000000000..00df54077 --- /dev/null +++ b/Core/PeekabooAutomationKit/Tests/PeekabooAutomationKitTests/SameProcessWindowCloseTests.swift @@ -0,0 +1,39 @@ +import Foundation +import Testing +@testable import PeekabooAutomationKit + +@Suite( + .serialized, + .enabled(if: ProcessInfo.processInfo.environment["RUN_AUTOMATION_ACTIONS"]?.lowercased() == "true")) +struct SameProcessWindowCloseTests { + @Test + func `background close keeps host callbacks on MainActor and preserves sibling windows`() throws { + let fixtureURL = Bundle(for: FixtureBundleAnchor.self).bundleURL + .deletingLastPathComponent() + .appendingPathComponent("HostWindowCloseFixture") + try #require(FileManager.default.isExecutableFile(atPath: fixtureURL.path)) + + let outputURL = FileManager.default.temporaryDirectory + .appendingPathComponent("peekaboo-host-window-close-\(UUID().uuidString).log") + try #require(FileManager.default.createFile(atPath: outputURL.path, contents: Data())) + defer { try? FileManager.default.removeItem(at: outputURL) } + let output = try FileHandle(forWritingTo: outputURL) + defer { try? output.close() } + + let process = Process() + process.executableURL = fixtureURL + process.standardOutput = output + process.standardError = output + try process.run() + try DockService.waitForProcessExit(process, timeoutSeconds: 25) + + let diagnostics = try String(contentsOf: outputURL, encoding: .utf8) + #expect(process.terminationReason == .exit, "\(diagnostics)") + #expect(process.terminationStatus == 0, "\(diagnostics)") + #expect( + diagnostics.split(separator: "\n").contains("HOST_WINDOW_CLOSE_FIXTURE_COMPLETED"), + "The AppKit child must finish its assertions before exiting. \(diagnostics)") + } +} + +private final class FixtureBundleAnchor: NSObject {} diff --git a/Core/PeekabooAutomationKit/Tests/Support/HostWindowCloseFixture/HostWindowCloseFixture.swift b/Core/PeekabooAutomationKit/Tests/Support/HostWindowCloseFixture/HostWindowCloseFixture.swift new file mode 100644 index 000000000..b67dd947f --- /dev/null +++ b/Core/PeekabooAutomationKit/Tests/Support/HostWindowCloseFixture/HostWindowCloseFixture.swift @@ -0,0 +1,163 @@ +import AppKit +import Darwin +import PeekabooAutomationKit + +@main +private enum HostWindowCloseFixture { + @MainActor + static func main() { + DispatchQueue.global().asyncAfter(deadline: .now() + 20) { + FileHandle.standardError.write(Data("Host window close fixture timed out.\n".utf8)) + exit(124) + } + + let application = NSApplication.shared + let delegate = FixtureApplicationDelegate() + application.delegate = delegate + application.setActivationPolicy(.accessory) + withExtendedLifetime(delegate) { + application.run() + } + FileHandle.standardError.write(Data("AppKit run loop returned before fixture completion.\n".utf8)) + exit(1) + } +} + +@MainActor +private final class FixtureApplicationDelegate: NSObject, NSApplicationDelegate { + private var targetWindow: NSWindow? + + func applicationDidFinishLaunching(_: Notification) { + Task { @MainActor in + do { + try await self.closeHostWindow() + FileHandle.standardOutput.write(Data("HOST_WINDOW_CLOSE_FIXTURE_COMPLETED\n".utf8)) + exit(0) + } catch { + FileHandle.standardError.write(Data("Host window close fixture failed: \(error)\n".utf8)) + exit(1) + } + } + } + + func applicationShouldTerminate(_: NSApplication) -> NSApplication.TerminateReply { + .terminateCancel + } + + func applicationShouldTerminateAfterLastWindowClosed(_: NSApplication) -> Bool { + false + } + + private func closeHostWindow() async throws { + let service = WindowManagementService() + self.targetWindow = self.makeWindow(title: "Close target", origin: CGPoint(x: 100, y: 100)) + let sibling = self.makeWindow(title: "Keep open", origin: CGPoint(x: 500, y: 100)) + let targetDelegate = CloseDelegate() + let siblingDelegate = CloseDelegate() + targetDelegate.onClosed = { [weak self] in self?.targetWindow = nil } + self.targetWindow?.delegate = targetDelegate + sibling.delegate = siblingDelegate + defer { + self.targetWindow?.delegate = nil + self.targetWindow?.close() + self.targetWindow = nil + sibling.delegate = nil + sibling.close() + } + + let identity = try await self.captureWindowIdentity(self.targetWindow) + let siblingIdentity = try await self.captureWindowIdentity(sibling) + + do { + let result = try await service.closeWindowActionResult( + target: .windowId(identity.windowID), + expectedIdentity: identity, + allowForegroundFallback: false) + guard result.outcome?.state == .confirmedChange, + result.outcome?.delivery == .init(mechanism: .accessibilityAction, mode: .background) + else { + throw FixtureFailure( + "Close did not report a confirmed background AX action: \(String(describing: result.outcome))") + } + } catch { + let current = SystemIdentityResolver.windowIdentity(CGWindowID(identity.windowID)) + throw FixtureFailure( + "\(error); receipt=\(identity); current=\(String(describing: current)); " + + "retained=\(self.targetWindow != nil); closes=\(targetDelegate.closeCount)") + } + guard targetDelegate.closeCount == 1, + self.targetWindow == nil, + SystemIdentityResolver.windowIdentity(CGWindowID(identity.windowID)) == nil + else { + throw FixtureFailure("Target did not disappear after its owner released the closed window") + } + guard siblingDelegate.closeCount == 0, + sibling.isVisible, + SystemIdentityResolver.validateWindowMutationIdentity(siblingIdentity) + else { + throw FixtureFailure("Closing the target changed its sibling") + } + } + + private func captureWindowIdentity(_ window: NSWindow?) async throws -> WindowMutationIdentity { + // End this reference's lifetime before close so the delegate can release the WindowServer row. + guard let window else { throw FixtureFailure("Fixture window is missing") } + try await self.waitForWindowRegistration(window) + guard let windowID = CGWindowID(exactly: window.windowNumber), + let identity = SystemIdentityResolver.windowMutationIdentity(windowID: windowID), + identity.ownerProcessIdentifier == ProcessInfo.processInfo.processIdentifier, + window.isVisible + else { + throw FixtureFailure("Could not capture a visible host-owned window") + } + return identity + } + + private func waitForWindowRegistration(_ window: NSWindow) async throws { + guard let windowID = CGWindowID(exactly: window.windowNumber) else { + throw FixtureFailure("Window has no WindowServer ID") + } + let deadline = ContinuousClock.now.advanced(by: .seconds(2)) + // WindowServer initially publishes zero bounds even after orderFront returns. + while ContinuousClock.now < deadline { + if SystemIdentityResolver.windowIdentity(windowID)?.bounds.size == window.frame.size { + return + } + try await Task.sleep(for: .milliseconds(10)) + } + throw FixtureFailure("WindowServer did not publish the fixture's window frame") + } + + private func makeWindow(title: String, origin: CGPoint) -> NSWindow { + let window = NSWindow( + contentRect: CGRect(origin: origin, size: CGSize(width: 320, height: 240)), + styleMask: [.titled, .closable], + backing: .buffered, + defer: false) + window.title = title + window.isReleasedWhenClosed = false + window.animationBehavior = .none + window.orderFront(nil) + return window + } +} + +@MainActor +private final class CloseDelegate: NSObject, NSWindowDelegate { + private(set) var closeCount = 0 + var onClosed: (@MainActor () -> Void)? + + func windowWillClose(_: Notification) { + MainActor.preconditionIsolated() + self.closeCount += 1 + self.onClosed?() + } +} + +private struct FixtureFailure: Error, CustomStringConvertible { + let description: String + + init(_ description: String) { + self.description = description + } +} diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 578ef8735..bd4b83515 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -102,6 +102,10 @@ optional focus/identity probes fail closed, while optional AX identifier failure After successful setup, the synchronous scope attempts to reset to zero (not the previous timeout); reset failure overrides the operation's result or error. Cancellation must be thrown by the operation. Detached raw AX workers retain unchecked `AXChildWindowMessagingTimeout` scopes so their blocking calls stay off MainActor. +Background window close, minimized restore, maximize, and close verification use that remote route only for other +processes. A window owned by the current host stays on MainActor because in-process AX messaging synchronously +invokes AppKit and its delegates. Receipt validation stays unchanged; cross-process AX messaging deadlines cannot +bound synchronous in-process AppKit callbacks. ## Presentation and verification diff --git a/docs/commands/window.md b/docs/commands/window.md index 182023854..f90850a32 100644 --- a/docs/commands/window.md +++ b/docs/commands/window.md @@ -32,7 +32,8 @@ read_when: - Partially applied (frame changed but missed the request): the command still succeeds, `requested_bounds` and a `warning` string are included in the JSON payload, and the text output prints the actual frame plus the warning. - Fully ignored (frame did not change at all): the command fails with exit code 1 and error code `WINDOW_MANIPULATION_ERROR`, because reporting success would silently lie to scripts. Typical cause: shrinking a window below its minimum size when it already sits at that minimum. - If the frame cannot be re-read after the operation, the command succeeds with a `warning` that the reported bounds may be stale. -- `maximize` is background-safe geometry, not macOS full screen and not a green-button toggle. Peekaboo pins the selected exact window ID, chooses the screen with the greatest overlap, and applies that screen's visible frame with a 750 ms per-message AX deadline off MainActor. It never activates the app, enters full screen, or switches Spaces. +- `maximize` is background-safe geometry, not macOS full screen and not a green-button toggle. Peekaboo pins the selected exact window ID, chooses the screen with the greatest overlap, and applies that screen's visible frame with a 750 ms per-message AX deadline for remote apps. It never activates the app, enters full screen, or switches Spaces. +- Background close, minimized restore, maximize, and close verification run remote AX messages off MainActor. When the exact target belongs to the current Peekaboo host, the same operations run on MainActor because in-process AX calls synchronously invoke AppKit and its window delegates. Exact-window receipts and remote message deadlines remain unchanged. - `maximize` verifies the exact WindowServer frame for up to two seconds and fails rather than claiming success when the app ignores or constrains the request. A window already at the target visible frame is an idempotent no-op. The CLI then reads the exact ID back until its frame is stable before emitting `new_bounds`. - `focus` routes through the exact CG window ID, makes the window main, raises it, and honors the global focus flags (`--space-switch` to jump Spaces, `--bring-to-current-space` to move the window instead, etc.). Success requires macOS Accessibility to report that exact window as focused and Workspace to report its app as frontmost. - `focus --verify` performs a second command-level check against the exact focused window ID. A merely topmost/renderable sibling no longer counts as focused. diff --git a/tests/background-capability-guidance.test.mjs b/tests/background-capability-guidance.test.mjs index af2e74651..aed8c89ba 100644 --- a/tests/background-capability-guidance.test.mjs +++ b/tests/background-capability-guidance.test.mjs @@ -30,7 +30,7 @@ test('press guidance preserves the snapshot-pinned background route', () => { assert.match(press, /Background-only Agent\/MCP.*explicit fresh exact non-dialog snapshot/s); }); -const isTargetedRawPress = (line) => /^peekaboo press\b.*--(?:app|pid)\b/.test(line); +const isTargetedRawPress = (line) => /^(?:peekaboo|"\$PB") press\b.*--(?:app|pid)\b/.test(line); const hasSafeRawPressRoute = (line) => /--(?:foreground|snapshot|window-(?:id|title|index))\b/.test(line); @@ -68,6 +68,7 @@ test('bundled skill never advertises app/PID-only background press', () => { } assert.equal(isTargetedRawPress('peekaboo press return --pid 1234'), true); + assert.equal(isTargetedRawPress('"$PB" press return --pid 1234'), true); assert.equal(hasSafeRawPressRoute('peekaboo press return --pid 1234'), false); assert.equal(hasSafeRawPressRoute('peekaboo press return --pid 1234 --window-id 42'), true); }); @@ -75,12 +76,11 @@ test('bundled skill never advertises app/PID-only background press', () => { test('bundled skill keeps routine management examples read-only', () => { const skill = read('skills/peekaboo/SKILL.md'); - assert.doesNotMatch(skill, /^peekaboo clipboard (?:set|clear|restore)\b/m); - assert.doesNotMatch(skill, /^peekaboo permissions request\b/m); - assert.doesNotMatch(skill, /^peekaboo app focus\b/m); - assert.match(skill, /^peekaboo clipboard get --json$/m); - assert.match(skill, /^peekaboo permissions status --all-sources --json$/m); - assert.match(skill, /^peekaboo app list --include-hidden --include-background --json$/m); + assert.doesNotMatch(skill, /^(?:peekaboo|"\$PB") clipboard (?:set|clear|restore)\b/m); + assert.doesNotMatch(skill, /^(?:peekaboo|"\$PB") permissions request\b/m); + assert.doesNotMatch(skill, /^(?:peekaboo|"\$PB") app focus\b/m); + assert.match(skill, /^(?:peekaboo|"\$PB") permissions status --all-sources --json$/m); + assert.match(skill, /^(?:peekaboo|"\$PB") app list --include-hidden --include-background --json$/m); }); test('background Agent type guidance requires an explicit non-dialog snapshot', () => {