diff --git a/.github/workflows/ios.yml b/.github/workflows/ios.yml index ecb5fc3b54..3715c42844 100644 --- a/.github/workflows/ios.yml +++ b/.github/workflows/ios.yml @@ -154,6 +154,7 @@ jobs: -xctestrun "$XCTESTRUN_PATH" \ -destination "platform=iOS Simulator,id=${{ steps.ios-simulator.outputs.simulator-udid }}" \ -only-testing:AgentDeviceRunnerUITests/RunnerTests/testSinglePointerFlingFallsBackToXCTestCoordinateDragWhenPrivateSynthesisFails \ + -only-testing:AgentDeviceRunnerUITests/RunnerTests/testRecordStartThrowsTheCaptureRefusalItReceived \ -only-testing:AgentDeviceRunnerUITests/RunnerTests/testAlertDispatchResolvesItsOwnModalWithoutCoordinateTapRoutingProbe \ -only-testing:AgentDeviceRunnerUITests/RunnerTests/testAlertResolutionCannotBypassRequestedDeadline \ -only-testing:AgentDeviceRunnerUITests/RunnerTests/testTypeWithoutResolvedInputReturnsTypedFailureBeforeDispatchingText \ diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerAppScreenCapture.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerAppScreenCapture.swift index 9429ad3e0a..e44934777f 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerAppScreenCapture.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerAppScreenCapture.swift @@ -54,6 +54,21 @@ enum RunnerAppScreenCaptureFailure: String, Error { } } +extension RunnerTests { + /// The target rule, kept apart from the two queries so the rule itself is testable: an unresolved + /// window asks the system surface, and nothing else does. + func selectObservedScreenCapture( + resolving: () -> Result, + fallingBack: () -> Result + ) -> Result { + let outcome = resolving() + if case .failure(.unresolvedWindow) = outcome { + return fallingBack() + } + return outcome + } +} + #if canImport(UIKit) && os(iOS) extension RunnerTests { /// Captures the display hosting `app` instead of `XCUIScreen.main`. @@ -103,19 +118,6 @@ extension RunnerTests { ) } - /// The target rule, kept apart from the two queries so the rule itself is testable: an unresolved - /// window asks the system surface, and nothing else does. - func selectObservedScreenCapture( - resolving: () -> Result, - fallingBack: () -> Result - ) -> Result { - let outcome = resolving() - if case .failure(.unresolvedWindow) = outcome { - return fallingBack() - } - return outcome - } - private static func resolveCapturedAppScreen( app: XCUIApplication ) -> Result { @@ -140,6 +142,12 @@ extension RunnerTests { guard let cgImage = runnerCGImage(from: upright) else { return .failure(.unrenderableImage) } + // A zero-pixel image is a capture that did not happen, not a tiny one. Refusing it here — at the + // type that owns the fact — keeps a required consumer (a recording sizing its writer from this + // frame) from mistaking it for a usable frame and falling back to an untyped error (#2728). + guard cgImage.width > 0, cgImage.height > 0 else { + return .failure(.unrenderableImage) + } return .success( CapturedAppScreen( image: upright, diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift index e23dd7952f..e731ad4e2c 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift @@ -1466,13 +1466,14 @@ extension RunnerTests { fps: command.fps.map { Int32($0) } ) try recorder.start { [weak self] in - return self?.captureRunnerFrame(app: activeApp) + guard let self else { return .failure(.unresolvedScreen) } + return self.captureRunnerFrameResult(app: activeApp) } activeRecording = recorder return Response(ok: true, data: DataPayload(message: "recording started")) } catch { activeRecording = nil - return Response(ok: false, error: ErrorPayload(message: "failed to start recording: \(error.localizedDescription)")) + return Response(ok: false, error: Self.recordingStartErrorPayload(for: error)) } case .recordStop: guard let recorder = activeRecording else { @@ -2120,11 +2121,20 @@ extension RunnerTests { ) #endif case .back, .backInApp: - if tapInAppBackControl(app: activeApp) { + switch tapInAppBackControl(app: activeApp) { + case .performed: let message = command.command == .back ? "back" : "backInApp" return Response(ok: true, data: DataPayload(message: message)) + case .unavailable: + return Response( + ok: false, + error: ErrorPayload(message: "in-app back control is not available") + ) + case .unverified(let error): + // The fallback gesture ran but the display refused to be sampled. Reporting the typed refusal + // keeps an unknown outcome from being laundered into a definitive "no back control" (#2728). + return Response(ok: false, error: error) } - return Response(ok: false, error: ErrorPayload(message: "in-app back control is not available")) case .backSystem: if performSystemBackAction(app: activeApp) { return Response(ok: true, data: DataPayload(message: "backSystem")) diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Lifecycle.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Lifecycle.swift index 67c689cb1c..a74851b5ef 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Lifecycle.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Lifecycle.swift @@ -42,31 +42,59 @@ enum RunnerInteractionIdleWaits { extension RunnerTests { // MARK: - Recording - /// One frame for the recording pump and the keyboard settle sample. + /// One frame for a caller that tolerates a dropped one — keyboard settling, which skips a sample it + /// cannot take and keeps polling. A frame that must exist goes through `captureRunnerFrameResult`, + /// which says why it refused. /// /// On iOS the frame comes from the display owning a window, because a foldable's /// `XCUIScreen.main` can be the dark outer panel while the app runs on the inner one — a stream of - /// identical black frames would then read as a settled screen and as a finished recording (#2728). - /// An observation with no session window falls to the system surface's window, which is what the - /// home screen is. macOS keeps recording the host display the way it always has. + /// identical black frames would then read as a settled screen (#2728). An observation with no + /// session window falls to the system surface's window, which is what the home screen is. macOS + /// keeps the host display it always recorded. func captureRunnerFrame(app: XCUIApplication) -> RunnerImage? { -#if os(iOS) - guard case .success(let captured) = captureObservedScreen(app: app) else { + switch captureRunnerFrameResult(app: app) { + case .success(let captured): + return captured.image + case .failure: return nil } - return captured.image + } + + /// The same frame as `captureRunnerFrame`, but carrying the reason it refused, so a required first + /// frame — a recording's bootstrap, which sizes the whole writer from it — fails closed with a + /// typed code rather than a message. The ongoing pump reads the same result and ignores a refusal + /// the way it ignored the `nil` it used to get; only a frame that must exist owes a reason (#2728). + func captureRunnerFrameResult( + app: XCUIApplication + ) -> Result { +#if os(iOS) + return captureObservedScreen(app: app) #else - var image: RunnerImage? + var outcome: Result = .failure( + .unrenderableImage + ) let capture = { - let screenshot = XCUIScreen.main.screenshot() - image = screenshot.image + let image = XCUIScreen.main.screenshot().image + if let cgImage = runnerCGImage(from: image) { + // The host display has no resolved-panel facts to report; the recorder reads only the image + // and its pixel size, so these two are inert placeholders, not measurements the host scales by. + outcome = .success( + CapturedAppScreen( + image: image, + displayID: 0, + pixelWidth: cgImage.width, + pixelHeight: cgImage.height, + pixelsPerPoint: 1 + ) + ) + } } if Thread.isMainThread { capture() } else { DispatchQueue.main.sync(execute: capture) } - return image + return outcome #endif } diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Navigation.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Navigation.swift index 98b41d3bb5..650d7559bb 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Navigation.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Navigation.swift @@ -4,27 +4,66 @@ extension RunnerTests { static let navigationBackKeywords = ["back", "close", "cancel"] static let navigationFallbackVerificationDelay: TimeInterval = 0.25 - func tapInAppBackControl(app: XCUIApplication) -> Bool { + /// What one in-app `back` attempt concluded. `.unverified` carries the typed capture failure the + /// display refused with, so an optional visual check that could not run reports an unknown instead + /// of the false "no back control exists" that a `nil` sample would otherwise become (#2728). + enum InAppBackOutcome { + case performed + case unavailable + case unverified(ErrorPayload) + } + + /// The three answers a before/after visual comparison can give. `unobserved` is not evidence of + /// "no change": a display that refuses to be sampled proves nothing either way (#2728). + enum NavigationVisualObservation: Equatable { + case changed + case unchanged + case unobserved + + var logToken: String { + switch self { + case .changed: return "yes" + case .unchanged: return "no" + case .unobserved: return "unknown" + } + } + } + + /// One navigation-fallback capture: the encoded frame when the display answered, and the typed + /// reason it refused otherwise. Only iOS produces a refusal; other platforms capture nothing here. + struct NavigationVisualSample { + let data: Data? + let refusalCode: String? + let refusalHint: String? + + init(data: Data?, refusalCode: String? = nil, refusalHint: String? = nil) { + self.data = data + self.refusalCode = refusalCode + self.refusalHint = refusalHint + } + } + + func tapInAppBackControl(app: XCUIApplication) -> InAppBackOutcome { #if os(macOS) if let back = macOSNavigationBackElement(app: app) { tapElementCenter(app: app, element: back) - return true + return .performed } - return false + return .unavailable #elseif os(tvOS) _ = pressTvRemote(.menu) - return true + return .performed #else let buttons = app.navigationBars.buttons.allElementsBoundByIndex if let back = buttons.first(where: { $0.isHittable }) { back.tap() - return true + return .performed } if isSnapshotXCTestChannelPenalized(bundleId: currentBundleId) { NSLog("AGENT_DEVICE_RUNNER_IN_APP_BACK_SKIPPED_XCTEST_ENUMERATION bundle=%@", currentBundleId ?? "") } else if let back = topNavigationBackElement(app: app) { tapElementCenter(app: app, element: back) - return true + return .performed } return tapTopLeadingNavigationFallback(app: app) #endif @@ -109,11 +148,11 @@ extension RunnerTests { return CGPoint(x: frame.minX + xOffset, y: frame.minY + yOffset) } - private func tapTopLeadingNavigationFallback(app: XCUIApplication) -> Bool { + private func tapTopLeadingNavigationFallback(app: XCUIApplication) -> InAppBackOutcome { #if os(iOS) let frame = onScreenWindowFrame(app: app) guard let point = Self.topLeadingNavigationFallbackPoint(in: frame) else { - return false + return .unavailable } let before = captureNavigationFallbackVisualState(app: app) let context = synthesizedCoordinateContext( @@ -124,54 +163,135 @@ extension RunnerTests { synthesizedTapAt(app: app, x: point.x, y: point.y, context: context) } if case .performed = synthesized.outcome { - return didNavigationFallbackChangeVisualState(app: app, before: before) + return verifyNavigationFallbackOutcome(app: app, before: before) } let fallback = performGesture(app) { tapAt(app: app, x: point.x, y: point.y) } if case .performed = fallback.outcome { - return didNavigationFallbackChangeVisualState(app: app, before: before) + return verifyNavigationFallbackOutcome(app: app, before: before) } #endif - return false + return .unavailable } - private func captureNavigationFallbackVisualState(app: XCUIApplication) -> Data? { + private func captureNavigationFallbackVisualState(app: XCUIApplication) -> NavigationVisualSample { #if os(iOS) - // A visual check is optional evidence: a display that owns no window reports unknown by returning - // no sample, which the comparison below already reads as "nothing proved". It never reaches for - // a screen nobody is on, whose stable black would then be read as evidence that the tap did - // nothing (#2728). - guard case .success(let captured) = captureObservedScreen(app: app) else { - return nil - } - return runnerPngData(for: captured.image) + return Self.navigationFallbackSample( + resolvingApp: { self.captureResolvedAppScreen(app: app) }, + systemSurface: { self.captureResolvedAppScreen(app: self.springboard) }, + encoding: { runnerPngData(for: $0.image) } + ) #else - return nil + return NavigationVisualSample(data: nil) #endif } - private func didNavigationFallbackChangeVisualState( + /// The decision the in-app `back` fallback makes about WHAT to sample, kept apart from the live app + /// so the decision itself is testable. It samples only the app's own resolved screen: consulting the + /// system surface for an app that resolved no window would capture SpringBoard's home screen, which + /// reads as "unchanged" across a before/after pair and launders a wrong-process frame into a false + /// "no back control" — the opposite of the unknown-outcome answer the fallback owes (#2728). The + /// system surface is still threaded in, so sampling it is the tested contract: a caller that started + /// to consult it fails the fallback's test. `navigationVisualSample` then names the refusal. + static func navigationFallbackSample( + resolvingApp: () -> Result, + systemSurface: () -> Result, + encoding: (CapturedAppScreen) -> Data? + ) -> NavigationVisualSample { + _ = systemSurface + return navigationVisualSample(from: resolvingApp(), encoding: encoding) + } + + /// Turns a capture answer into a navigation sample: the encoded frame when the display answered, + /// and the typed reason it refused otherwise. A resolved display whose image would not encode is + /// named as the capture failure it is, not as an unnamed no-sample that would default to "no + /// display resolved" (#2728). Kept apart from the query so the mapping itself is testable. + static func navigationVisualSample( + from outcome: Result, + encoding: (CapturedAppScreen) -> Data? + ) -> NavigationVisualSample { + switch outcome { + case .success(let captured): + guard let png = encoding(captured) else { + let refusal = RunnerAppScreenCaptureFailure.unrenderableImage + return NavigationVisualSample( + data: nil, + refusalCode: refusal.rawValue, + refusalHint: refusal.hint + ) + } + return NavigationVisualSample(data: png) + case .failure(let failure): + return NavigationVisualSample( + data: nil, + refusalCode: failure.rawValue, + refusalHint: failure.hint + ) + } + } + + private func verifyNavigationFallbackOutcome( app: XCUIApplication, - before: Data? - ) -> Bool { + before: NavigationVisualSample + ) -> InAppBackOutcome { sleepFor(Self.navigationFallbackVerificationDelay) let after = captureNavigationFallbackVisualState(app: app) - let changed = Self.didNavigationFallbackChangeVisualState(before: before, after: after) - // The sample sizes are what tells a refused capture apart from an unchanged screen, and the - // fallback is rare enough that saying so every time costs nothing (#2728). + let observation = Self.navigationVisualObservation(before: before.data, after: after.data) + // The sample sizes and the observation name together tell a refused capture apart from an + // unchanged screen, and the fallback is rare enough that saying so every time costs nothing. NSLog( "AGENT_DEVICE_RUNNER_IN_APP_BACK_VISUAL_VERIFICATION beforeBytes=%ld afterBytes=%ld changed=%@", - before?.count ?? -1, - after?.count ?? -1, - changed ? "yes" : "no" + before.data?.count ?? -1, + after.data?.count ?? -1, + observation.logToken ) - return changed + return Self.inAppBackOutcome(observation: observation, before: before, after: after) + } + + /// What an observation of the fallback's before/after samples concludes. Kept apart from the sleep + /// and the capture so the three-way decision is testable without a running app. + static func inAppBackOutcome( + observation: NavigationVisualObservation, + before: NavigationVisualSample, + after: NavigationVisualSample + ) -> InAppBackOutcome { + switch observation { + case .changed: + return .performed + case .unchanged: + return .unavailable + case .unobserved: + // No sample is not evidence of no navigation change: the fallback ran, so report the display + // refusal it hit rather than laundering an unobservable result into "back is not available". + return .unverified(Self.navigationFallbackErrorPayload(after: after, before: before)) + } } - static func didNavigationFallbackChangeVisualState(before: Data?, after: Data?) -> Bool { - guard let before, let after else { return false } - return before != after + static func navigationVisualObservation( + before: Data?, + after: Data? + ) -> NavigationVisualObservation { + guard let before, let after else { return .unobserved } + return before != after ? .changed : .unchanged + } + + /// The refusal the fallback most recently hit wins, so the code names the last thing it looked at + /// before giving up. A missing reason on both sides is unreachable on iOS (every nil sample carries + /// one) and defaults to the plain display-unresolved code. + static func navigationFallbackErrorPayload( + after: NavigationVisualSample, + before: NavigationVisualSample + ) -> ErrorPayload { + ErrorPayload( + code: + after.refusalCode + ?? before.refusalCode + ?? RunnerAppScreenCaptureFailure.unresolvedScreen.rawValue, + message: + "The in-app back fallback was dispatched, but no display could be sampled to confirm the result. This is an unknown outcome, not evidence that a back control is absent.", + hint: after.refusalHint ?? before.refusalHint + ) } private func macOSNavigationBackElement(app: XCUIApplication) -> XCUIElement? { @@ -234,21 +354,119 @@ extension RunnerTests { XCTAssertFalse(Self.isTopNavigationControlFrame(.infinite, in: window)) } - func testNavigationFallbackRequiresObservedVisualChange() { - XCTAssertTrue( - Self.didNavigationFallbackChangeVisualState( - before: Data([1, 2, 3]), - after: Data([1, 2, 4]) - ) + func testNavigationVisualVerificationSeparatesNoChangeFromNoSample() { + XCTAssertEqual( + Self.navigationVisualObservation(before: Data([1, 2, 3]), after: Data([1, 2, 4])), + .changed ) - XCTAssertFalse( - Self.didNavigationFallbackChangeVisualState( - before: Data([1, 2, 3]), - after: Data([1, 2, 3]) - ) + XCTAssertEqual( + Self.navigationVisualObservation(before: Data([1, 2, 3]), after: Data([1, 2, 3])), + .unchanged + ) + // A missing sample is neither a change nor a no-change; treating it as "unchanged" would let a + // capture that refused become the reason the `back` command claims no control exists (#2728). + XCTAssertEqual(Self.navigationVisualObservation(before: nil, after: Data([1])), .unobserved) + XCTAssertEqual(Self.navigationVisualObservation(before: Data([1]), after: nil), .unobserved) + XCTAssertEqual(Self.navigationVisualObservation(before: nil, after: nil), .unobserved) + } + + func testNavigationFallbackReportsTheRefusalItHitNotADefaultCode() { + // The refusal from the most recent sample wins, so the code names what the fallback last looked at + // before giving up; an earlier refusal is reported only when the later sample carried none (#2728). + let after = NavigationVisualSample( + data: nil, + refusalCode: "APP_SCREEN_WINDOW_UNRESOLVED", + refusalHint: "after hint" + ) + let before = NavigationVisualSample( + data: nil, + refusalCode: "APP_SCREEN_UNRESOLVED", + refusalHint: "before hint" + ) + let laterWins = Self.navigationFallbackErrorPayload(after: after, before: before) + XCTAssertEqual(laterWins.code, "APP_SCREEN_WINDOW_UNRESOLVED") + XCTAssertEqual(laterWins.hint, "after hint") + + let onlyBefore = Self.navigationFallbackErrorPayload( + after: NavigationVisualSample(data: nil), + before: before + ) + XCTAssertEqual(onlyBefore.code, "APP_SCREEN_UNRESOLVED") + XCTAssertEqual(onlyBefore.hint, "before hint") + + // Neither side named a reason (unreachable on iOS): a real capture code, never a bare failure. + let unnamed = Self.navigationFallbackErrorPayload( + after: NavigationVisualSample(data: nil), + before: NavigationVisualSample(data: nil) + ) + XCTAssertEqual(unnamed.code, "APP_SCREEN_UNRESOLVED") + XCTAssertTrue(unnamed.message.contains("unknown outcome")) + } + + func testVerifyNavigationFallbackOutcomeReportsUnresolvedWindowWithoutSystemSurface() { + // The in-app `back` fallback ran its tap but the app resolved no window. It must name that refusal + // as an unknown outcome AND must not have sampled the system surface: capturing SpringBoard's home + // screen twice reads as "unchanged" and launders a wrong-process frame into "no back control + // exists" (#2728). This drives the SAME `navigationFallbackSample` production calls, handing it a + // system surface that fails the test if consulted — so reverting the fallback to sample SpringBoard + // (or dropping the refusal code) turns this red, which an inline `.never` re-creation could not. + var askedSystemSurface = false + let sample = Self.navigationFallbackSample( + resolvingApp: { .failure(.unresolvedWindow) }, + systemSurface: { + askedSystemSurface = true + return .failure(.unresolvedWindow) + }, + encoding: { _ in Data([1, 2, 3]) } + ) + XCTAssertFalse(askedSystemSurface, "the in-app fallback samples the app only, never SpringBoard") + + XCTAssertNil(sample.data) + XCTAssertEqual(sample.refusalCode, "APP_SCREEN_WINDOW_UNRESOLVED") + + let observation = Self.navigationVisualObservation(before: sample.data, after: sample.data) + XCTAssertEqual(observation, .unobserved) + + switch Self.inAppBackOutcome(observation: observation, before: sample, after: sample) { + case .unverified(let payload): + XCTAssertEqual(payload.code, "APP_SCREEN_WINDOW_UNRESOLVED") + case .performed, .unavailable: + XCTFail("a refused capture must report an unknown outcome, not 'no back control'") + } + } + + func testNavigationVisualSampleDistinguishesEncodedFrameFromRefusal() { + // The same capture entry point yields three different samples, and only the refusal ones may carry + // a code: an encoded frame is evidence, a resolved-but-unencodable image and a refusal are not + // (#2728). Reverting the mapping to a plain no-sample loses the reason a host keys on. + let captured = CapturedAppScreen( + image: RunnerImage(), + displayID: 3, + pixelWidth: 12, + pixelHeight: 24, + pixelsPerPoint: 3 + ) + + let encoded = Self.navigationVisualSample( + from: .success(captured), + encoding: { _ in Data([7, 7]) } + ) + XCTAssertEqual(encoded.data, Data([7, 7])) + XCTAssertNil(encoded.refusalCode) + + let unencodable = Self.navigationVisualSample( + from: .success(captured), + encoding: { _ in nil } + ) + XCTAssertNil(unencodable.data) + XCTAssertEqual(unencodable.refusalCode, "APP_SCREEN_CAPTURE_UNRENDERABLE") + + let refused = Self.navigationVisualSample( + from: .failure(.unresolvedScreen), + encoding: { _ in Data([7, 7]) } ) - XCTAssertFalse(Self.didNavigationFallbackChangeVisualState(before: nil, after: Data([1]))) - XCTAssertFalse(Self.didNavigationFallbackChangeVisualState(before: Data([1]), after: nil)) + XCTAssertNil(refused.data) + XCTAssertEqual(refused.refusalCode, "APP_SCREEN_UNRESOLVED") } #endif } diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+ScreenRecorder.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+ScreenRecorder.swift index 6f55379c24..a7e13154fe 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+ScreenRecorder.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+ScreenRecorder.swift @@ -30,7 +30,9 @@ extension RunnerTests { self.fps = fps } - func start(captureFrame: @escaping () -> RunnerImage?) throws { + func start( + capture: @escaping () -> Result + ) throws { let url = URL(fileURLWithPath: outputPath) let directory = url.deletingLastPathComponent() try FileManager.default.createDirectory( @@ -44,21 +46,34 @@ extension RunnerTests { var dimensions: CGSize = .zero var bootstrapImage: RunnerImage? + var lastFailure: RunnerAppScreenCaptureFailure? let bootstrapDeadline = Date().addingTimeInterval(2.0) while Date() < bootstrapDeadline { - if let image = captureFrame(), let cgImage = runnerCGImage(from: image) { - bootstrapImage = image - dimensions = CGSize(width: cgImage.width, height: cgImage.height) + switch capture() { + case .success(let captured): + bootstrapImage = captured.image + dimensions = CGSize(width: captured.pixelWidth, height: captured.pixelHeight) + case .failure(let failure): + lastFailure = failure + } + if dimensions.width > 0, dimensions.height > 0 { break } Thread.sleep(forTimeInterval: 0.05) } guard dimensions.width > 0, dimensions.height > 0 else { - throw NSError( - domain: "AgentDeviceRunner.Record", - code: 1, - userInfo: [NSLocalizedDescriptionKey: "failed to capture initial frame"] - ) + // The bootstrap frame is required: the writer is sized from it. A capture that refused names + // why (no window, no display, unencodable image) so the host sees a typed reason rather than + // the generic "no frame" it used to collapse every refusal into (#2728). macOS keeps its + // host-display behavior and its original error, because nothing here is a panel question. + #if os(iOS) + throw RunnerTests.recordingBootstrapError(from: lastFailure) + #else + // macOS/tvOS preserve their original untyped record error regardless of why the host capture + // refused; the reason is read here only so the shared bootstrap loop carries no dead write. + _ = lastFailure + throw RunnerTests.recordingBootstrapError(from: nil) + #endif } let writer = try AVAssetWriter(outputURL: url, fileType: .mp4) @@ -114,8 +129,8 @@ extension RunnerTests { timer.setEventHandler { [weak self] in guard let self else { return } if self.shouldStop() { return } - guard let image = captureFrame() else { return } - self.append(image: image) + guard case .success(let captured) = capture() else { return } + self.append(image: captured.image) } self.timer = timer timer.resume() @@ -269,6 +284,31 @@ extension RunnerTests { } } +extension RunnerTests { + /// The error a `record start` bootstrap raises when no initial frame arrived. On iOS the last capture + /// refusal (if any) is the honest reason and travels as its own typed code; only when nothing + /// refused — a macOS host capture, or a deadline that elapsed before any answer — does it fall back + /// to the original untyped record error, which keeps pre-panel behavior intact (#2728). + static func recordingBootstrapError(from lastFailure: RunnerAppScreenCaptureFailure?) -> Error { + lastFailure + ?? NSError( + domain: "AgentDeviceRunner.Record", + code: 1, + userInfo: [NSLocalizedDescriptionKey: "failed to capture initial frame"] + ) + } + + /// Maps a `record start` failure to the wire payload. A capture that refused carries a typed + /// `APP_SCREEN_*` reason, so a no-window bootstrap reaches the host as that code rather than the + /// generic record error it used to collapse into; a genuine writer failure keeps its message. + static func recordingStartErrorPayload(for error: Error) -> ErrorPayload { + if let failure = error as? RunnerAppScreenCaptureFailure { + return ErrorPayload(code: failure.rawValue, message: failure.message, hint: failure.hint) + } + return ErrorPayload(message: "failed to start recording: \(error.localizedDescription)") + } +} + #if AGENT_DEVICE_RUNNER_UNIT_TESTS extension RunnerTests.ScreenRecorder { @discardableResult diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+RecordingTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+RecordingTests.swift index 8875716a6e..ce8bd2c2c5 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+RecordingTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+RecordingTests.swift @@ -23,5 +23,76 @@ extension RunnerTests { // The helper records each returned value as an accepted frame. XCTAssertEqual(recorder.allocateTimestampForTesting(0), 101) } + + func testRecordingBootstrapErrorKeepsTheTypedRefusalAndFallsBackGenerically() { + // A capture that refused is the honest bootstrap error; nothing refusing keeps the pre-panel + // untyped record error, so the iOS typed path and the macOS generic path are both pinned (#2728). + let typed = RunnerTests.recordingBootstrapError(from: .unresolvedWindow) + XCTAssertEqual( + (typed as? RunnerAppScreenCaptureFailure)?.rawValue, + "APP_SCREEN_WINDOW_UNRESOLVED" + ) + + let generic = RunnerTests.recordingBootstrapError(from: nil) + XCTAssertNil(generic as? RunnerAppScreenCaptureFailure) + XCTAssertEqual((generic as NSError).code, 1) + } + + func testRecordingStartSurfacesACaptureRefusalAsATypedCode() { + // The bootstrap frame is required, so a no-window refusal must travel as its own code rather than + // the generic record error it used to collapse into; a real writer failure keeps its message (#2728). + let refusal = RunnerTests.recordingStartErrorPayload( + for: RunnerAppScreenCaptureFailure.unresolvedWindow + ) + XCTAssertEqual(refusal.code, "APP_SCREEN_WINDOW_UNRESOLVED") + XCTAssertNotNil(refusal.hint) + + let writerFailure = NSError( + domain: "AgentDeviceRunner.Record", + code: 5, + userInfo: [NSLocalizedDescriptionKey: "failed to append frame"] + ) + let generic = RunnerTests.recordingStartErrorPayload(for: writerFailure) + XCTAssertNil(generic.code) + XCTAssertTrue(generic.message.contains("failed to start recording")) + XCTAssertTrue(generic.message.contains("failed to append frame")) + } #endif } + +#if AGENT_DEVICE_RUNNER_UNIT_TESTS && os(iOS) +extension RunnerTests { + func testRecordStartThrowsTheCaptureRefusalItReceived() throws { + // The bootstrap frame is required, so `record start` must surface the exact refusal its capture + // saw rather than the generic "failed to capture initial frame" every refusal used to collapse + // into, and that refusal must reach the host as its own code. Driving `start` with an always- + // refusing capture and mapping the ACTUAL thrown error — not a re-typed literal — proves the + // bootstrap forwards its last refusal end to end; the pure mapping tests cannot catch that wiring. + let outputPath = (NSTemporaryDirectory() as NSString).appendingPathComponent( + "record-refusal-\(UUID().uuidString).mp4" + ) + let recorder = ScreenRecorder(outputPath: outputPath, fps: 30) + + var captureCalls = 0 + var thrown: Error? + XCTAssertThrowsError( + try recorder.start(capture: { + captureCalls += 1 + return .failure(.unresolvedWindow) + }) + ) { error in + thrown = error + XCTAssertEqual( + (error as? RunnerAppScreenCaptureFailure)?.rawValue, + "APP_SCREEN_WINDOW_UNRESOLVED", + "the thrown bootstrap error is the refusal the capture returned" + ) + } + XCTAssertGreaterThan(captureCalls, 0, "the bootstrap must poll the injected capture") + + // The refusal the bootstrap actually threw maps to its own host code, not the generic record error. + let payload = RunnerTests.recordingStartErrorPayload(for: try XCTUnwrap(thrown)) + XCTAssertEqual(payload.code, "APP_SCREEN_WINDOW_UNRESOLVED") + } +} +#endif diff --git a/docs/adr/0025-foldable-apple-panels.md b/docs/adr/0025-foldable-apple-panels.md index acc21d64ee..4acc358aa7 100644 --- a/docs/adr/0025-foldable-apple-panels.md +++ b/docs/adr/0025-foldable-apple-panels.md @@ -110,6 +110,21 @@ resolves a window. An app with no window is refused on the query's own `exists` its frame: a windowless app answers `frame` with `(0,0 0x0)` without raising, so geometry alone cannot say "no window". +A visual *check* on an app's own transition is a different question, and the system surface's second +answer would fabricate it. The `back` fallback's before/after comparison asks whether THIS app's +screen changed under a leading tap the app itself received; two SpringBoard captures of the home +screen are byte-identical no matter what the app did, so a system-surface sample would report +"unchanged" for exactly the failure it is meant to catch. The check therefore observes the app's own +resolved window and nothing else: an app that resolves no window reports the typed refusal as an +unknown outcome, not as the false no-change a second process would produce (#2728). + +Required and optional consumers of the same helper split on the frame they are owed. `screenshot` and +`record start`'s bootstrap frame are required — the recording sizes its whole writer from that first +image — and each names why the capture did not happen with the shared `APP_SCREEN_*` reason, so a +no-window runtime is a typed failure and never a generic one. The keyboard stability pump and the +recording pump keep a nil-tolerant frame, because a dropped frame between polls is normal and says +nothing about which display a window is on (#2728). + Both answers are measured rather than assumed. A session with no app, and one whose app was terminated while still bound, answer the window query with `resolved=no`, and the capture that follows names the lit panel — on an unfolded Duo that is `LCD-1`, and the resulting home-screen image contains no black pixel. An app suspended by `home` keeps answering with a usable window, so it takes the first branch and still captures the panel it is on. `screenshot` is a runner-lifecycle command and skips the app-activation preflight, which left its diff --git a/packages/platform-apple/src/runner/runner-contract.ts b/packages/platform-apple/src/runner/runner-contract.ts index aabcd91638..605415ff89 100644 --- a/packages/platform-apple/src/runner/runner-contract.ts +++ b/packages/platform-apple/src/runner/runner-contract.ts @@ -696,11 +696,15 @@ export function shouldRestartRunnerBeforeCommandSend(error: unknown): boolean { export const SCROLL_KEYBOARD_OCCLUDES_SURFACE_RUNNER_CODE = 'SCROLL_KEYBOARD_OCCLUDES_SURFACE'; /** - * The codes the XCTest runner answers with when a screenshot did not happen (#2728): no window - * resolved so no display could be named, the resolved window named no display, or the resolved - * display handed back an image it could not encode upright. They are the runner's own vocabulary, so - * they are declared here beside the set that keeps them off the wire, and a required capture fails - * closed on them rather than falling back to a screen nobody is on. + * The codes the XCTest runner answers with when a resolved-display capture it was asked to make did + * not happen (#2728): no window resolved so no display could be named, the resolved window named no + * display, or the resolved display handed back an image it could not encode upright. The runner emits + * them from every consumer of that helper — the `screenshot` command's fallback, `record start`'s + * required first frame, and an optional visual check such as the `back` fallback's before/after + * sample. They are the runner's own vocabulary, so they are declared here beside the set that keeps + * them off the wire; a required capture fails closed on them rather than falling back to a screen + * nobody is on, and an optional one reports the refusal as an unknown. Only the `screenshot` route + * consumes the set for its own simctl-to-runner decision, but the set names the whole family. */ const RUNNER_SCREEN_WINDOW_UNRESOLVED_RUNNER_CODE = 'APP_SCREEN_WINDOW_UNRESOLVED'; const RUNNER_SCREEN_UNRESOLVED_RUNNER_CODE = 'APP_SCREEN_UNRESOLVED';