From 13641ec7f8d8f372a50af46d643ab52fd458bfcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 22 Oct 2026 06:21:12 +0200 Subject: [PATCH 1/5] fix(ios): prevent XCTest channel penalty recovery loops --- .../RunnerTests+SnapshotCapturePlan.swift | 3 +- .../RunnerTests+SnapshotTiming.swift | 41 ++++++++-- ...ts+SnapshotCapturePlanOccupancyTests.swift | 54 +++++-------- ...RunnerTests+SnapshotCapturePlanTests.swift | 38 +++++++++ .../RunnerTests+SnapshotTimingTests.swift | 80 +++++++++++++++++-- .../adr/0004-ios-snapshot-backend-strategy.md | 11 ++- 6 files changed, 179 insertions(+), 48 deletions(-) diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift index a0be484d79..b00c77c049 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift @@ -386,7 +386,8 @@ extension RunnerTests { kind, attempt: attempt, bundleId: target.bundleId, - penaltySuppressed: suppressXCTestPenalty + penaltySuppressed: suppressXCTestPenalty, + channelState: effective.xCTestChannelState ) if case let .failed(failure, phase: _) = attempt.outcome { if Self.isAxSnapshotFailure(failure) { axFailure = failure } diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotTiming.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotTiming.swift index 3a3cfa03a6..40514405b4 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotTiming.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotTiming.swift @@ -119,9 +119,12 @@ extension RunnerTests { static func snapshotXCTestPenaltyReason( kind: SnapshotBackendKind, attempt: SnapshotBackendAttempt, - slowThresholdMs: Double + slowThresholdMs: Double, + channelState: SnapshotXCTestChannelPlanState = .normal ) -> String? { - guard kind.usesXCTestAccessibilityChannel else { return nil } + guard channelState == .normal, kind.usesXCTestAccessibilityChannel else { return nil } + let isSlow = attempt.timing.acquisitionMs > slowThresholdMs + guard kind == .recursiveTree || isSlow else { return nil } if case let .failed(failure, phase: .acquisition) = attempt.outcome, failure.code == Self.xCTestSnapshotTimeoutCode { @@ -130,22 +133,50 @@ extension RunnerTests { if attempt.tierOutcome == .deadlineExhausted { return "\(kind.rawValue)_backend_timeout" } - guard attempt.timing.acquisitionMs > slowThresholdMs else { return nil } + guard isSlow else { return nil } return "slow_\(kind.rawValue)_capture_\(Int(attempt.timing.acquisitionMs))ms" } + /// Only an accepted, fast tree proves that a bounded recovery probe restored the channel. + static func snapshotXCTestRecoveryProbeSucceeded( + kind: SnapshotBackendKind, + attempt: SnapshotBackendAttempt, + channelState: SnapshotXCTestChannelPlanState + ) -> Bool { + guard channelState != .normal, + kind == .recursiveTree, + attempt.timing.acquisitionMs <= Self.penalizedXCTestProbeTreeSliceBudget * 1_000, + case let .captured(capture) = attempt.outcome + else { return false } + return Self.snapshotTierRejectionReason( + outcome: attempt.tierOutcome, + kind: kind, + payload: capture.qualityPayload ?? capture.payload + ) == nil + } + func recordXCTestSnapshotBackendAttemptIfNeeded( _ kind: SnapshotBackendKind, attempt: SnapshotBackendAttempt, bundleId: String?, - penaltySuppressed: Bool + penaltySuppressed: Bool, + channelState: SnapshotXCTestChannelPlanState ) { guard !penaltySuppressed else { return } + if Self.snapshotXCTestRecoveryProbeSucceeded( + kind: kind, + attempt: attempt, + channelState: channelState + ) { + clearSnapshotXCTestChannelPenalty(reason: "healthy_tree_recovery_probe") + return + } guard let reason = Self.snapshotXCTestPenaltyReason( kind: kind, attempt: attempt, - slowThresholdMs: snapshotXCTestSlowCaptureThreshold * 1_000 + slowThresholdMs: snapshotXCTestSlowCaptureThreshold * 1_000, + channelState: channelState ) else { return } penalizeSnapshotXCTestChannel( diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanOccupancyTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanOccupancyTests.swift index 55f3c0416f..122398f116 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanOccupancyTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanOccupancyTests.swift @@ -298,12 +298,9 @@ extension RunnerTests { XCTAssertFalse(hasAbandonedMainThreadWork()) } - /// A query sweep that stops at its own slice deadline collected a partial tree, not an answer, so - /// it is a tier timeout: the plan must arm the XCTest-channel penalty and let private AX answer, - /// keeping the partial sweep only as the fallback. Accepting it because it carries more nodes than - /// the sparse threshold ships a hierarchy-free capture and leaves every later capture of the same - /// screen to pay for the full sweep again (#2781). - func testQuerySweepThatEndsOnItsSliceDeadlinePenalizesChannelAndReachesPrivateAX() throws { + /// An incomplete sweep must still reach private AX (#2781), but its short slice is insufficient + /// evidence for a channel penalty (#3844). + func testQuerySweepDeadlineRecoversWithoutPenalizingChannel() throws { guard let queryMethod = class_getInstanceMethod( XCUIElementQuery.self, @@ -345,9 +342,6 @@ extension RunnerTests { var error: Error? var abandonedAtReturn = false var penalizedAtReturn = false - var sweepStartsAfterFirstPlan = 0 - var secondPayload: DataPayload? - var secondError: Error? } let box = ResultBox() let planned = expectation(description: "slow-sweep plan answered") @@ -365,18 +359,6 @@ extension RunnerTests { } box.abandonedAtReturn = self.hasAbandonedMainThreadWork() box.penalizedAtReturn = self.isSnapshotXCTestChannelPenalized(bundleId: captureTarget.bundleId) - box.sweepStartsAfterFirstPlan = RunnerSlowSweepQueryGate.recordedStarts().count - do { - box.secondPayload = try self.runSnapshotCapturePlan( - Self.regularVisiblePlan, - target: captureTarget, - options: PresentationOptions(interactiveOnly: false, depth: nil, scope: nil, raw: false), - terminal: .sparseWithFatalOnAXFailure, - deadline: Date().addingTimeInterval(20) - ) - } catch { - box.secondError = error - } planned.fulfill() } @@ -391,21 +373,9 @@ extension RunnerTests { XCTAssertEqual(quality?.state, .recovered) XCTAssertGreaterThan(box.payload?.nodes?.count ?? 0, 1, "private AX answers with a real tree") XCTAssertFalse(box.abandonedAtReturn, "the sweep must answer inside its own main-thread hop") - XCTAssertTrue( + XCTAssertFalse( box.penalizedAtReturn, - "the tier timeout must arm the XCTest-channel penalty for the captured target" - ) - - XCTAssertNil(box.secondError) - XCTAssertEqual( - box.secondPayload?.snapshotQuality?.backend, - SnapshotBackendKind.privateAX.rawValue, - "the armed penalty must defer the next plan's XCTest tiers, not re-run the sweep" - ) - XCTAssertEqual( - RunnerSlowSweepQueryGate.recordedStarts().count, - box.sweepStartsAfterFirstPlan, - "the deferred plan may start no sweep query at all" + "a single short query-sweep timeout must not arm the channel penalty" ) XCTAssertFalse(hasAbandonedMainThreadWork()) } @@ -430,6 +400,14 @@ extension RunnerTests { XCTAssertEqual(quality.state, .recovered) XCTAssertEqual(quality.backend, SnapshotBackendKind.recursiveTree.rawValue) XCTAssertGreaterThan(payload.nodes?.count ?? 0, 1, "the bounded tree answers with a real tree") + let timing = try XCTUnwrap(quality.timing) + guard timing.acquisitionMs <= Self.penalizedXCTestProbeTreeSliceBudget * 1_000 else { + throw XCTSkip("the accepted tree's total acquisition exceeded the fast-probe threshold on this host") + } + XCTAssertFalse( + isSnapshotXCTestChannelPenalized(bundleId: captureTarget.bundleId), + "an accepted fast tree probe must clear the existing penalty early" + ) } /// When the bounded tree also grinds past its slice, the plan ends sparse, and the verdict still @@ -465,9 +443,15 @@ extension RunnerTests { tearDownPenalizedPrivateAXBlindTarget() } + let originalPenaltyUntil = snapshotXCTestChannelPenaltyUntil let payload = try runDeferredPlanOffMain(target: captureTarget) { RunnerBlockingSnapshotGate.release.signal() } + XCTAssertEqual( + snapshotXCTestChannelPenaltyUntil, + originalPenaltyUntil, + "the bounded recovery probe must not renew its own penalty when its tree work is abandoned" + ) let quality = try XCTUnwrap(payload.snapshotQuality) XCTAssertEqual(quality.state, .sparse) diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanTests.swift index d8ce8065fb..cae3a09b87 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanTests.swift @@ -612,5 +612,43 @@ extension RunnerTests { XCTAssertLessThanOrEqual(nodes.count, unscoped.count) } #endif + + func testOnlyFastAcceptedTreeProbeClearsPenalty() { + let payload = DataPayload(nodes: [ + planTestNode(index: 0, type: "Application", label: "App"), + planTestNode(index: 1, type: "Button", label: "Open", hittable: true, parentIndex: 0), + ], truncated: false) + let healthy = SnapshotBackendCapture(payload: payload, effectiveDepth: nil) + let fast = SnapshotBackendAttempt( + outcome: .captured(healthy), + timing: SnapshotCaptureTiming(acquisitionMs: 100, presentationMs: 10) + ) + for state in [SnapshotXCTestChannelPlanState.deferredToIndependentBackend, .boundedXCTestProbe] { + XCTAssertTrue(Self.snapshotXCTestRecoveryProbeSucceeded( + kind: .recursiveTree, attempt: fast, channelState: state + )) + for kind in [SnapshotBackendKind.querySweep, .privateAX] { + XCTAssertFalse(Self.snapshotXCTestRecoveryProbeSucceeded( + kind: kind, attempt: fast, channelState: state + )) + } + let rejected = [ + SnapshotBackendAttempt(outcome: .captured(healthy), timing: fast.timing, tierOutcome: .deadlineExhausted), + SnapshotBackendAttempt(outcome: .noCapture, timing: fast.timing), + SnapshotBackendAttempt(outcome: .captured(healthy), timing: SnapshotCaptureTiming(acquisitionMs: 1_001, presentationMs: 0)), + SnapshotBackendAttempt(outcome: .captured(SnapshotBackendCapture( + payload: payload, effectiveDepth: nil, qualityPayload: DataPayload(nodes: [], truncated: false) + )), timing: fast.timing), + ] + for attempt in rejected { + XCTAssertFalse(Self.snapshotXCTestRecoveryProbeSucceeded( + kind: .recursiveTree, attempt: attempt, channelState: state + )) + } + } + XCTAssertFalse(Self.snapshotXCTestRecoveryProbeSucceeded( + kind: .recursiveTree, attempt: fast, channelState: .normal + )) + } } #endif diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotTimingTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotTimingTests.swift index 822f8dbab9..cd12b23e36 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotTimingTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotTimingTests.swift @@ -60,10 +60,7 @@ extension RunnerTests { ) } - /// A tier that stopped starting work at its own deadline is a timeout for the breaker even when it - /// answered fast and with a payload: the partial sweep must arm the penalty exactly as a slow tier - /// does, and a tier that finished fast must not (#2781). - func testDeadlineExhaustedTierAttemptArmsTheChannelPenaltyWithoutBeingSlow() { + func testDeadlineExhaustedTreeAttemptArmsTheChannelPenaltyWithoutBeingSlow() { let capture = SnapshotBackendCapture( payload: DataPayload(nodes: [], truncated: true), effectiveDepth: nil @@ -75,11 +72,11 @@ extension RunnerTests { ) XCTAssertEqual( Self.snapshotXCTestPenaltyReason( - kind: .querySweep, + kind: .recursiveTree, attempt: exhausted, slowThresholdMs: 3_000 ), - "queries_backend_timeout" + "tree_backend_timeout" ) let completed = SnapshotBackendAttempt( @@ -122,5 +119,76 @@ extension RunnerTests { XCTAssertEqual(timer.timing.acquisitionMs, 2_000, accuracy: 0.001) XCTAssertEqual(timer.timing.presentationMs, 5_000, accuracy: 0.001) } + + func testShortSweepTimeoutDoesNotPenalizeButSlowSweepStillDoes() { + let timeout = SnapshotCaptureFailure( + code: Self.xCTestSnapshotTimeoutCode, message: "same timeout text", hint: "test" + ) + for outcome in [SnapshotBackendAttempt.Outcome.failed(timeout, phase: .acquisition), .noCapture] { + let short = SnapshotBackendAttempt( + outcome: outcome, + timing: SnapshotCaptureTiming(acquisitionMs: 1_000, presentationMs: 10), + tierOutcome: .deadlineExhausted + ) + XCTAssertNil(Self.snapshotXCTestPenaltyReason( + kind: .querySweep, attempt: short, slowThresholdMs: 3_000 + )) + XCTAssertEqual(Self.snapshotXCTestPenaltyReason( + kind: .recursiveTree, attempt: short, slowThresholdMs: 3_000 + ), "tree_backend_timeout") + let slow = SnapshotBackendAttempt( + outcome: outcome, + timing: SnapshotCaptureTiming(acquisitionMs: 3_001, presentationMs: 10), + tierOutcome: .deadlineExhausted + ) + XCTAssertEqual(Self.snapshotXCTestPenaltyReason( + kind: .querySweep, attempt: slow, slowThresholdMs: 3_000 + ), "queries_backend_timeout") + } + let untyped = SnapshotBackendAttempt( + outcome: .failed(SnapshotCaptureFailure( + code: "OTHER_FAILURE", message: "same timeout text", hint: "test" + ), phase: .acquisition), + timing: SnapshotCaptureTiming(acquisitionMs: 100, presentationMs: 0) + ) + XCTAssertNil(Self.snapshotXCTestPenaltyReason( + kind: .recursiveTree, attempt: untyped, slowThresholdMs: 3_000 + )) + } + + func testBoundedRecoveryAttemptsCannotRenewPenaltyOnEitherPlatformPlan() { + let timeout = SnapshotCaptureFailure( + code: Self.xCTestSnapshotTimeoutCode, message: "test timeout", hint: "test" + ) + let attempts = [ + SnapshotBackendAttempt( + outcome: .failed(timeout, phase: .acquisition), + timing: SnapshotCaptureTiming(acquisitionMs: 1_000, presentationMs: 0) + ), + SnapshotBackendAttempt( + outcome: .noCapture, + timing: SnapshotCaptureTiming(acquisitionMs: 1_000, presentationMs: 0), + tierOutcome: .deadlineExhausted + ), + SnapshotBackendAttempt( + outcome: .noCapture, + timing: SnapshotCaptureTiming(acquisitionMs: 3_001, presentationMs: 0) + ), + ] + for state in [SnapshotXCTestChannelPlanState.deferredToIndependentBackend, .boundedXCTestProbe] { + for kind in [SnapshotBackendKind.recursiveTree, .querySweep] { + for attempt in attempts { + XCTAssertNil(Self.snapshotXCTestPenaltyReason( + kind: kind, attempt: attempt, slowThresholdMs: 3_000, channelState: state + )) + } + } + } + for attempt in attempts { + XCTAssertNotNil(Self.snapshotXCTestPenaltyReason( + kind: .recursiveTree, attempt: attempt, slowThresholdMs: 3_000 + ), "ordinary tree timeouts and slow acquisitions must still penalize") + } + } } #endif diff --git a/docs/adr/0004-ios-snapshot-backend-strategy.md b/docs/adr/0004-ios-snapshot-backend-strategy.md index 69c8373437..a8d57ee058 100644 --- a/docs/adr/0004-ios-snapshot-backend-strategy.md +++ b/docs/adr/0004-ios-snapshot-backend-strategy.md @@ -231,13 +231,22 @@ On heavy or animating screens, the recursive tree capture can work for many seco This pushes the chained plan past the runner's main-thread watchdog and fills the main queue with retries. The plan now carries its umbrella deadline into the query-sweep and private-AX tiers. When the budget is spent, later ladder rungs stop. -A slow, timed-out, or watchdog-abandoned XCTest-backed capture penalizes the XCTest accessibility channel for that bundle for a bounded window. +A slow XCTest acquisition, a timed-out tree capture, or watchdog-abandoned system-modal work penalizes the XCTest accessibility channel for that bundle for a bounded window. Subsequent regular plans derive the next step from backend traits (`effectiveSnapshotCapturePlan`). If a runnable non-XCTest backend exists, they defer to that independent tier. If it does not exist, as on physical iOS devices today, they run a short XCTest probe instead of the full tree slice. Thus, healthy screens can recover without another long capture of the hostile screen. The raw diagnostic plan is exempt and keeps tree-first error propagation. +Issue #3844 showed that the bounded recovery probe could renew its own penalty. +A short query-sweep timeout could also start another window after the previous window expired. +A bounded recovery probe now leaves the penalty deadline unchanged when it fails. +Its short slice tests recovery under a known penalty, so its timeout supplies no new evidence about the channel. +A completed tree probe clears the penalty early only when its acquisition fits the probe slice and its quality is accepted. +A short query-sweep timeout still rejects the partial capture and continues recovery, but it does not penalize the channel. +A slow sweep remains penalty evidence because it exceeded the acquisition threshold. +The plan deadline and abandoned-main-thread-work gate remain in force on every attempt, including physical-device probes (#1105, #1156, #2621). + A third shape followed on the same app class after the plan recovered reliably. On that app class, each of the query-sweep tier's 19 `allElementsBoundByIndex` reads fails with `kAXErrorIllegalArgument`. XCTest records every one as a test failure worded `Failed to resolve query: ...`. From 96d03c4bc55033a1f19b33de87a175f3fce16eff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 22 Oct 2026 07:20:04 +0200 Subject: [PATCH 2/5] fix(ios): schedule bounded recovery without losing hostile-tree deferral --- .../RunnerTests+SnapshotCapturePlan.swift | 86 ++++++++++++++++++- .../RunnerTests+SnapshotTiming.swift | 1 - .../RunnerTests.swift | 1 + ...ts+SnapshotCapturePlanOccupancyTests.swift | 52 +++++++++-- ...RunnerTests+SnapshotCapturePlanTests.swift | 42 +++++++++ .../RunnerTests+SnapshotTimingTests.swift | 6 +- .../adr/0004-ios-snapshot-backend-strategy.md | 14 ++- 7 files changed, 188 insertions(+), 14 deletions(-) diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift index b00c77c049..c50830e122 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift @@ -145,7 +145,9 @@ extension RunnerTests { func penalizeSnapshotXCTestChannel(bundleId: String?, reason: String) { snapshotXCTestChannelPenaltyLock.lock() snapshotXCTestChannelPenaltyBundleId = bundleId - snapshotXCTestChannelPenaltyUntil = Date().addingTimeInterval(snapshotXCTestChannelPenaltyDuration) + let now = Date() + snapshotXCTestChannelPenaltyUntil = now.addingTimeInterval(snapshotXCTestChannelPenaltyDuration) + snapshotXCTestChannelNextRecoveryProbeAt = now.addingTimeInterval(Self.snapshotXCTestRecoveryProbeInterval) snapshotXCTestChannelPenaltyLock.unlock() NSLog( "AGENT_DEVICE_RUNNER_SNAPSHOT_XCTEST_CHANNEL_PENALIZED bundle=%@ reason=%@", @@ -159,6 +161,7 @@ extension RunnerTests { let hadActivePenalty = Date() < snapshotXCTestChannelPenaltyUntil snapshotXCTestChannelPenaltyBundleId = nil snapshotXCTestChannelPenaltyUntil = Date.distantPast + snapshotXCTestChannelNextRecoveryProbeAt = Date.distantFuture snapshotXCTestChannelPenaltyLock.unlock() if hadActivePenalty { NSLog("AGENT_DEVICE_RUNNER_SNAPSHOT_XCTEST_CHANNEL_PENALTY_CLEARED reason=%@", reason) @@ -295,6 +298,80 @@ extension RunnerTests { ) } + static let snapshotXCTestRecoveryProbeInterval: TimeInterval = 30 + + static func shouldScheduleSnapshotXCTestRecoveryProbe( + effective: EffectiveSnapshotCapturePlan, + acceptedBackend: SnapshotBackendKind, + now: Date, + nextProbeAt: Date, + deadline: Date, + hasAbandonedWork: Bool + ) -> Bool { + effective.xCTestChannelState == .deferredToIndependentBackend + && effective.preferredBackend == nil + && effective.plan.contains(.recursiveTree) + && !acceptedBackend.usesXCTestAccessibilityChannel + && !hasAbandonedWork + && now >= nextProbeAt + && deadline.timeIntervalSince(now) >= Self.penalizedXCTestProbeTreeSliceBudget + } + + private func runScheduledSnapshotXCTestRecoveryProbeIfNeeded( + effective: EffectiveSnapshotCapturePlan, + acceptedBackend: SnapshotBackendKind, + target: SnapshotCaptureTarget, + options: PresentationOptions, + deadline: Date + ) { + let abandoned = hasAbandonedMainThreadWork() + let now = Date() + snapshotXCTestChannelPenaltyLock.lock() + let penaltyMatches = now < snapshotXCTestChannelPenaltyUntil + && (snapshotXCTestChannelPenaltyBundleId == nil || snapshotXCTestChannelPenaltyBundleId == target.bundleId) + let scheduled = penaltyMatches && Self.shouldScheduleSnapshotXCTestRecoveryProbe( + effective: effective, + acceptedBackend: acceptedBackend, + now: now, + nextProbeAt: snapshotXCTestChannelNextRecoveryProbeAt, + deadline: deadline, + hasAbandonedWork: abandoned + ) + if scheduled { + snapshotXCTestChannelNextRecoveryProbeAt = now.addingTimeInterval(Self.snapshotXCTestRecoveryProbeInterval) + } + snapshotXCTestChannelPenaltyLock.unlock() + guard scheduled else { return } + let slice = Self.penalizedXCTestProbeTreeSliceBudget + NSLog("AGENT_DEVICE_RUNNER_SNAPSHOT_XCTEST_RECOVERY_PROBE_SCHEDULED bundle=%@", target.bundleId ?? "") + defer { + NSLog("AGENT_DEVICE_RUNNER_SNAPSHOT_XCTEST_RECOVERY_PROBE_FINISHED elapsedMs=%d", Int(Date().timeIntervalSince(now) * 1_000)) + } + // One idle-only hop bounds the entire optional probe, including viewport reads and processing. + // Its abandoned work uses the ordinary occupancy gate, but never arms a new penalty. + let attempt = try? runMainThreadWorkIfIdle( + "snapshot_recovery_probe", + timeout: slice, + timeoutError: snapshotMainThreadTimeoutError("probing XCTest recovery") + ) { + try self.captureWithBackend( + .recursiveTree, + target: target, + options: options, + deadline: now.addingTimeInterval(slice), + treeCaptureSliceBudgetOverride: slice + ) + } + guard let attempt else { return } + recordXCTestSnapshotBackendAttemptIfNeeded( + .recursiveTree, + attempt: attempt, + bundleId: target.bundleId, + penaltySuppressed: false, + channelState: effective.xCTestChannelState + ) + } + func shouldSkipSnapshotBackendForAbandonedMainThreadWork(_ kind: SnapshotBackendKind) -> Bool { kind.usesXCTestAccessibilityChannel && hasAbandonedMainThreadWork() } @@ -435,6 +512,13 @@ extension RunnerTests { verdictReason?.reason ?? "sparse tree" ) } + runScheduledSnapshotXCTestRecoveryProbeIfNeeded( + effective: effective, + acceptedBackend: kind, + target: target, + options: options, + deadline: deadline + ) return stampedSnapshotPayload( capture, backend: kind, diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotTiming.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotTiming.swift index 40514405b4..6f29ea4c4e 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotTiming.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotTiming.swift @@ -124,7 +124,6 @@ extension RunnerTests { ) -> String? { guard channelState == .normal, kind.usesXCTestAccessibilityChannel else { return nil } let isSlow = attempt.timing.acquisitionMs > slowThresholdMs - guard kind == .recursiveTree || isSlow else { return nil } if case let .failed(failure, phase: .acquisition) = attempt.outcome, failure.code == Self.xCTestSnapshotTimeoutCode { diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift index b334d7301d..c99fcaabe3 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift @@ -127,6 +127,7 @@ final class RunnerTests: XCTestCase { let snapshotXCTestChannelPenaltyLock = NSLock() var snapshotXCTestChannelPenaltyBundleId: String? var snapshotXCTestChannelPenaltyUntil = Date.distantPast + var snapshotXCTestChannelNextRecoveryProbeAt = Date.distantFuture let snapshotXCTestChannelPenaltyDuration: TimeInterval = 120 let snapshotXCTestPenaltyWarmupExemption = SnapshotXCTestPenaltyWarmupExemption() // Sticky per-bundle hint for the private AX depth ladder: deep RN screens reject the default diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanOccupancyTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanOccupancyTests.swift index 122398f116..7886353d84 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanOccupancyTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanOccupancyTests.swift @@ -298,9 +298,8 @@ extension RunnerTests { XCTAssertFalse(hasAbandonedMainThreadWork()) } - /// An incomplete sweep must still reach private AX (#2781), but its short slice is insufficient - /// evidence for a channel penalty (#3844). - func testQuerySweepDeadlineRecoversWithoutPenalizingChannel() throws { + /// An incomplete normal-plan sweep must reach private AX and penalize the hostile channel. + func testQuerySweepDeadlineRecoversAndPenalizesChannel() throws { guard let queryMethod = class_getInstanceMethod( XCUIElementQuery.self, @@ -373,9 +372,9 @@ extension RunnerTests { XCTAssertEqual(quality?.state, .recovered) XCTAssertGreaterThan(box.payload?.nodes?.count ?? 0, 1, "private AX answers with a real tree") XCTAssertFalse(box.abandonedAtReturn, "the sweep must answer inside its own main-thread hop") - XCTAssertFalse( + XCTAssertTrue( box.penalizedAtReturn, - "a single short query-sweep timeout must not arm the channel penalty" + "a normal-plan query-sweep timeout must arm the channel penalty" ) XCTAssertFalse(hasAbandonedMainThreadWork()) } @@ -521,6 +520,49 @@ extension RunnerTests { XCTAssertNotEqual(quality.state, .healthy, "a tree without its collapsed tab items is not healthy") } + func testScheduledRecoveryProbeAfterAcceptedPrivateAXIsBoundedAndCannotRepeatOrRearm() throws { + let captureTarget = try launchPenalizedPrivateAXBlindTarget( + bundleId: "com.callstack.agentdevice.runner.scheduled-probe-test" + ) + defer { tearDownPenalizedPrivateAXBlindTarget() } + let options = PresentationOptions(interactiveOnly: false, depth: nil, scope: nil, raw: false) + let acquisition = try XCTUnwrap(privateAXSnapshotAcquisition( + target: captureTarget, hint: SnapshotPresentation.captureHint(for: options), + deadline: Date().addingTimeInterval(10) + )) + privateAXAcquisitionOverrideForTesting = { acquisition } + let snapshotMethod = try XCTUnwrap(class_getInstanceMethod(XCUIApplication.self, #selector(XCUIElement.snapshot))) + let stubMethod = try XCTUnwrap(class_getInstanceMethod(RunnerBlockingSnapshotStub.self, #selector(RunnerBlockingSnapshotStub.snapshot))) + let original = method_getImplementation(snapshotMethod) + RunnerBlockingSnapshotGate.release = DispatchSemaphore(value: 0) + RunnerBlockingSnapshotGate.entered = DispatchSemaphore(value: 0) + method_setImplementation(snapshotMethod, method_getImplementation(stubMethod)) + defer { + RunnerBlockingSnapshotGate.release.signal() + method_setImplementation(snapshotMethod, original) + let deadline = Date().addingTimeInterval(3) + while hasAbandonedMainThreadWork(), Date() < deadline { + RunLoop.current.run(until: Date().addingTimeInterval(0.01)) + } + } + snapshotXCTestChannelNextRecoveryProbeAt = .distantPast + let expiry = snapshotXCTestChannelPenaltyUntil + let started = Date() + let payload = try runDeferredPlanOffMain(target: captureTarget) { + RunnerBlockingSnapshotGate.release.signal() + } + XCTAssertEqual(payload.snapshotQuality?.backend, SnapshotBackendKind.privateAX.rawValue) + XCTAssertEqual(RunnerBlockingSnapshotGate.entered.wait(timeout: .now()), .success) + XCTAssertLessThan(Date().timeIntervalSince(started), 2) + XCTAssertEqual(snapshotXCTestChannelPenaltyUntil, expiry) + let next = snapshotXCTestChannelNextRecoveryProbeAt + XCTAssertGreaterThan(next.timeIntervalSince(started), 29) + _ = try runDeferredPlanOffMain(target: captureTarget) + XCTAssertEqual(snapshotXCTestChannelNextRecoveryProbeAt, next) + XCTAssertEqual(snapshotXCTestChannelPenaltyUntil, expiry) + XCTAssertEqual(RunnerBlockingSnapshotGate.entered.wait(timeout: .now()), .timedOut) + } + private func launchPenalizedPrivateAXBlindTarget(bundleId: String) throws -> SnapshotCaptureTarget { app.launchArguments = ["--agent-device-selector-read-regression"] app.launch() diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanTests.swift index cae3a09b87..045ca2d6aa 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanTests.swift @@ -632,6 +632,11 @@ extension RunnerTests { kind: kind, attempt: fast, channelState: state )) } + XCTAssertTrue(Self.snapshotXCTestRecoveryProbeSucceeded( + kind: .recursiveTree, + attempt: SnapshotBackendAttempt(outcome: .captured(healthy), timing: SnapshotCaptureTiming(acquisitionMs: 1_000, presentationMs: 10)), + channelState: state + )) let rejected = [ SnapshotBackendAttempt(outcome: .captured(healthy), timing: fast.timing, tierOutcome: .deadlineExhausted), SnapshotBackendAttempt(outcome: .noCapture, timing: fast.timing), @@ -650,5 +655,42 @@ extension RunnerTests { kind: .recursiveTree, attempt: fast, channelState: .normal )) } + func testScheduledRecoveryProbeRequiresDueIndependentRegularPlanAndIdleBudget() { + let now = Date(timeIntervalSinceReferenceDate: 100) + let deferred = Self.effectiveSnapshotCapturePlan(Self.regularVisiblePlan, xCTestChannelPenalized: true) + func schedules( + _ effective: EffectiveSnapshotCapturePlan, + backend: SnapshotBackendKind = .privateAX, + next: Date = now, + remaining: TimeInterval = 1, + abandoned: Bool = false + ) -> Bool { + Self.shouldScheduleSnapshotXCTestRecoveryProbe( + effective: effective, acceptedBackend: backend, now: now, nextProbeAt: next, + deadline: now.addingTimeInterval(remaining), hasAbandonedWork: abandoned + ) + } + XCTAssertTrue(schedules(deferred)) + XCTAssertFalse(schedules(deferred, next: now.addingTimeInterval(0.001))) + XCTAssertFalse(schedules(deferred, remaining: 0.999)) + XCTAssertFalse(schedules(deferred, abandoned: true)) + XCTAssertFalse(schedules(deferred, backend: .recursiveTree)) + XCTAssertFalse(schedules(deferred, backend: .querySweep)) + XCTAssertFalse(schedules(Self.effectiveSnapshotCapturePlan( + Self.regularVisiblePlan, xCTestChannelPenalized: false + ))) + XCTAssertFalse(schedules(Self.effectiveSnapshotCapturePlan( + Self.rawDiagnosticPlan, xCTestChannelPenalized: true + ))) + XCTAssertFalse(schedules(Self.effectiveSnapshotCapturePlan( + Self.regularVisiblePlan, xCTestChannelPenalized: true, + availableBackends: [.recursiveTree, .querySweep] + ))) + XCTAssertFalse(schedules(Self.effectiveSnapshotCapturePlan( + Self.regularVisiblePlan, xCTestChannelPenalized: true, preferredBackend: "private-ax" + ))) + XCTAssertEqual(Self.snapshotXCTestRecoveryProbeInterval, 30) + } + } #endif diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotTimingTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotTimingTests.swift index cd12b23e36..82e8d238f4 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotTimingTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotTimingTests.swift @@ -120,7 +120,7 @@ extension RunnerTests { XCTAssertEqual(timer.timing.presentationMs, 5_000, accuracy: 0.001) } - func testShortSweepTimeoutDoesNotPenalizeButSlowSweepStillDoes() { + func testNormalPlanSweepTimeoutPenalizesRegardlessOfItsShortSlice() { let timeout = SnapshotCaptureFailure( code: Self.xCTestSnapshotTimeoutCode, message: "same timeout text", hint: "test" ) @@ -130,9 +130,9 @@ extension RunnerTests { timing: SnapshotCaptureTiming(acquisitionMs: 1_000, presentationMs: 10), tierOutcome: .deadlineExhausted ) - XCTAssertNil(Self.snapshotXCTestPenaltyReason( + XCTAssertEqual(Self.snapshotXCTestPenaltyReason( kind: .querySweep, attempt: short, slowThresholdMs: 3_000 - )) + ), "queries_backend_timeout") XCTAssertEqual(Self.snapshotXCTestPenaltyReason( kind: .recursiveTree, attempt: short, slowThresholdMs: 3_000 ), "tree_backend_timeout") diff --git a/docs/adr/0004-ios-snapshot-backend-strategy.md b/docs/adr/0004-ios-snapshot-backend-strategy.md index a8d57ee058..726564611d 100644 --- a/docs/adr/0004-ios-snapshot-backend-strategy.md +++ b/docs/adr/0004-ios-snapshot-backend-strategy.md @@ -231,7 +231,7 @@ On heavy or animating screens, the recursive tree capture can work for many seco This pushes the chained plan past the runner's main-thread watchdog and fills the main queue with retries. The plan now carries its umbrella deadline into the query-sweep and private-AX tiers. When the budget is spent, later ladder rungs stop. -A slow XCTest acquisition, a timed-out tree capture, or watchdog-abandoned system-modal work penalizes the XCTest accessibility channel for that bundle for a bounded window. +A slow XCTest acquisition, a timed-out XCTest tier, or watchdog-abandoned system-modal work penalizes the XCTest accessibility channel for that bundle for a bounded window. Subsequent regular plans derive the next step from backend traits (`effectiveSnapshotCapturePlan`). If a runnable non-XCTest backend exists, they defer to that independent tier. If it does not exist, as on physical iOS devices today, they run a short XCTest probe instead of the full tree slice. @@ -239,12 +239,18 @@ Thus, healthy screens can recover without another long capture of the hostile sc The raw diagnostic plan is exempt and keeps tree-first error propagation. Issue #3844 showed that the bounded recovery probe could renew its own penalty. -A short query-sweep timeout could also start another window after the previous window expired. A bounded recovery probe now leaves the penalty deadline unchanged when it fails. Its short slice tests recovery under a known penalty, so its timeout supplies no new evidence about the channel. A completed tree probe clears the penalty early only when its acquisition fits the probe slice and its quality is accepted. -A short query-sweep timeout still rejects the partial capture and continues recovery, but it does not penalize the channel. -A slow sweep remains penalty evidence because it exceeded the acquisition threshold. +After an independent backend succeeds, a penalized regular plan can schedule the existing bounded tree probe once per 30 seconds. +The interval starts when the penalty is armed. +The one-second idle-only probe bounds all acquisition and processing work. +The accepted independent capture remains the response. +The schedule excludes physical-device plans, explicit backend pins, occupied main-thread work, and plans with less than one second remaining. +The interval limits hostile-screen probe cost while giving healthy screens a recovery opportunity before penalty expiry. +A normal-plan query-sweep timeout rejects the partial capture and penalizes the channel. +Its short slice is evidence of a hostile tree, and deferral prevents repeated slow sweeps on that tree. +A normal-plan XCTest acquisition above the existing three-second threshold also remains penalty evidence. The plan deadline and abandoned-main-thread-work gate remain in force on every attempt, including physical-device probes (#1105, #1156, #2621). A third shape followed on the same app class after the plan recovered reliably. From 2fd5cd83aa8161ca33b4a2aa470ef433fec09134 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 22 Oct 2026 07:24:57 +0200 Subject: [PATCH 3/5] fix(ios): run recovery probes after returning accepted captures --- .../RunnerTests+SnapshotCapturePlan.swift | 32 ++++++++++++++++--- ...ts+SnapshotCapturePlanOccupancyTests.swift | 22 +++++++++++-- .../adr/0004-ios-snapshot-backend-strategy.md | 2 ++ 3 files changed, 50 insertions(+), 6 deletions(-) diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift index c50830e122..3fdb6d9573 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift @@ -317,7 +317,7 @@ extension RunnerTests { && deadline.timeIntervalSince(now) >= Self.penalizedXCTestProbeTreeSliceBudget } - private func runScheduledSnapshotXCTestRecoveryProbeIfNeeded( + private func scheduleSnapshotXCTestRecoveryProbeIfNeeded( effective: EffectiveSnapshotCapturePlan, acceptedBackend: SnapshotBackendKind, target: SnapshotCaptureTarget, @@ -338,10 +338,34 @@ extension RunnerTests { hasAbandonedWork: abandoned ) if scheduled { - snapshotXCTestChannelNextRecoveryProbeAt = now.addingTimeInterval(Self.snapshotXCTestRecoveryProbeInterval) + snapshotXCTestChannelNextRecoveryProbeAt = .distantFuture } + let penaltyUntil = snapshotXCTestChannelPenaltyUntil snapshotXCTestChannelPenaltyLock.unlock() guard scheduled else { return } + commandExecutionQueue.async { + self.snapshotXCTestChannelPenaltyLock.lock() + let stillCurrent = self.snapshotXCTestChannelPenaltyUntil == penaltyUntil + && Date() < penaltyUntil + if stillCurrent { + self.snapshotXCTestChannelNextRecoveryProbeAt = Date().addingTimeInterval(Self.snapshotXCTestRecoveryProbeInterval) + } + self.snapshotXCTestChannelPenaltyLock.unlock() + guard stillCurrent, !self.hasAbandonedMainThreadWork(), + deadline.timeIntervalSinceNow >= Self.penalizedXCTestProbeTreeSliceBudget + else { return } + self.runScheduledSnapshotXCTestRecoveryProbe( + target: target, options: options, channelState: effective.xCTestChannelState + ) + } + } + + private func runScheduledSnapshotXCTestRecoveryProbe( + target: SnapshotCaptureTarget, + options: PresentationOptions, + channelState: SnapshotXCTestChannelPlanState + ) { + let now = Date() let slice = Self.penalizedXCTestProbeTreeSliceBudget NSLog("AGENT_DEVICE_RUNNER_SNAPSHOT_XCTEST_RECOVERY_PROBE_SCHEDULED bundle=%@", target.bundleId ?? "") defer { @@ -368,7 +392,7 @@ extension RunnerTests { attempt: attempt, bundleId: target.bundleId, penaltySuppressed: false, - channelState: effective.xCTestChannelState + channelState: channelState ) } @@ -512,7 +536,7 @@ extension RunnerTests { verdictReason?.reason ?? "sparse tree" ) } - runScheduledSnapshotXCTestRecoveryProbeIfNeeded( + scheduleSnapshotXCTestRecoveryProbeIfNeeded( effective: effective, acceptedBackend: kind, target: target, diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanOccupancyTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanOccupancyTests.swift index 7886353d84..6388902f6e 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanOccupancyTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanOccupancyTests.swift @@ -548,9 +548,14 @@ extension RunnerTests { snapshotXCTestChannelNextRecoveryProbeAt = .distantPast let expiry = snapshotXCTestChannelPenaltyUntil let started = Date() - let payload = try runDeferredPlanOffMain(target: captureTarget) { + let payload = try runDeferredPlanOffMain(target: captureTarget) + XCTAssertLessThan(Date().timeIntervalSince(started), 0.5, "the accepted capture must answer before the optional probe") + let probeFinished = expectation(description: "scheduled probe answered") + commandExecutionQueue.async { RunnerBlockingSnapshotGate.release.signal() + probeFinished.fulfill() } + wait(for: [probeFinished], timeout: 3) XCTAssertEqual(payload.snapshotQuality?.backend, SnapshotBackendKind.privateAX.rawValue) XCTAssertEqual(RunnerBlockingSnapshotGate.entered.wait(timeout: .now()), .success) XCTAssertLessThan(Date().timeIntervalSince(started), 2) @@ -558,9 +563,22 @@ extension RunnerTests { let next = snapshotXCTestChannelNextRecoveryProbeAt XCTAssertGreaterThan(next.timeIntervalSince(started), 29) _ = try runDeferredPlanOffMain(target: captureTarget) + let queueDrained = expectation(description: "second capture queued no probe") + commandExecutionQueue.async { queueDrained.fulfill() } + wait(for: [queueDrained], timeout: 3) XCTAssertEqual(snapshotXCTestChannelNextRecoveryProbeAt, next) XCTAssertEqual(snapshotXCTestChannelPenaltyUntil, expiry) XCTAssertEqual(RunnerBlockingSnapshotGate.entered.wait(timeout: .now()), .timedOut) + + snapshotXCTestChannelNextRecoveryProbeAt = .distantPast + _ = try runDeferredPlanOffMain(target: captureTarget) { + self.clearSnapshotXCTestChannelPenalty(reason: "replace-before-probe") + self.penalizeSnapshotXCTestChannel(bundleId: captureTarget.bundleId, reason: "new-penalty") + } + let staleProbeDrained = expectation(description: "stale queued probe refused") + commandExecutionQueue.async { staleProbeDrained.fulfill() } + wait(for: [staleProbeDrained], timeout: 3) + XCTAssertEqual(RunnerBlockingSnapshotGate.entered.wait(timeout: .now()), .timedOut) } private func launchPenalizedPrivateAXBlindTarget(bundleId: String) throws -> SnapshotCaptureTarget { @@ -602,7 +620,7 @@ extension RunnerTests { } let box = ResultBox() let planned = expectation(description: "deferred plan answered") - DispatchQueue(label: "agent-device.runner.tests.deferred-plan").async { + commandExecutionQueue.async { do { box.payload = try self.runSnapshotCapturePlan( Self.regularVisiblePlan, diff --git a/docs/adr/0004-ios-snapshot-backend-strategy.md b/docs/adr/0004-ios-snapshot-backend-strategy.md index 726564611d..371086adcb 100644 --- a/docs/adr/0004-ios-snapshot-backend-strategy.md +++ b/docs/adr/0004-ios-snapshot-backend-strategy.md @@ -246,6 +246,8 @@ After an independent backend succeeds, a penalized regular plan can schedule the The interval starts when the penalty is armed. The one-second idle-only probe bounds all acquisition and processing work. The accepted independent capture remains the response. +The existing serial command queue runs the optional probe after that response. +A queued probe reserves the interval and checks that its penalty is still current before it starts. The schedule excludes physical-device plans, explicit backend pins, occupied main-thread work, and plans with less than one second remaining. The interval limits hostile-screen probe cost while giving healthy screens a recovery opportunity before penalty expiry. A normal-plan query-sweep timeout rejects the partial capture and penalizes the channel. From 092d21aa4a9248b69c8213502dee8243140a895f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 22 Oct 2026 10:33:05 +0200 Subject: [PATCH 4/5] fix(ios): run the due XCTest recovery probe inside the deferred capture Drop the post-response recovery probe: it started XCTest main-thread work after the snapshot had answered, so its abandoned tree work could refuse the next tap with RUNNER_BUSY while the snapshot's occupancy stamp said idle. The penalty is now one SnapshotXCTestChannelPenalty value (arm, clear, isProbeDue, claimProbe). When a probe is due, effectiveSnapshotCapturePlan puts the bounded tree ahead of private AX in the deferred plan, so any abandoned probe work is charged to that capture and its busy stamp. Restore the occupancy test's second-plan assertions that an armed penalty defers the next plan's XCTest tiers and starts no sweep query. Refs #3844 Co-Authored-By: Claude Opus 5.5 --- .../RunnerTests+AXSnapshotFallback.swift | 2 +- .../RunnerTests+SnapshotCapturePlan.swift | 199 +++++++----------- .../RunnerTests+SnapshotTiming.swift | 3 +- .../RunnerTests.swift | 5 +- ...ts+SnapshotCapturePlanOccupancyTests.swift | 118 +++++++---- ...RunnerTests+SnapshotCapturePlanTests.swift | 103 +++++---- .../RunnerTests+SnapshotTimingTests.swift | 9 +- 7 files changed, 224 insertions(+), 215 deletions(-) diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+AXSnapshotFallback.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+AXSnapshotFallback.swift index df933a24ab..d212491567 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+AXSnapshotFallback.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+AXSnapshotFallback.swift @@ -132,7 +132,7 @@ extension RunnerTests { privateAXAcceptedDepthBundleId = bundleId privateAXAcceptedDepthProcessIdentifier = processIdentifier privateAXAcceptedDepth = depth - privateAXAcceptedDepthUntil = Date().addingTimeInterval(snapshotXCTestChannelPenaltyDuration) + privateAXAcceptedDepthUntil = Date().addingTimeInterval(SnapshotXCTestChannelPenalty.duration) privateAXAcceptedDepthLock.unlock() NSLog("AGENT_DEVICE_RUNNER_PRIVATE_AX_DEPTH_REMEMBERED depth=%ld bundle=%@", depth, bundleId ?? "") } diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift index 3fdb6d9573..da71bebe7b 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift @@ -47,6 +47,45 @@ enum SnapshotXCTestChannelPlanState: Equatable { case boundedXCTestProbe } +/// The XCTest accessibility channel penalty (#1105/#1156). While it is active, regular plans for its +/// bundle defer XCTest tiers. Each recovery interval, one deferred plan runs the bounded tree probe +/// first, inside that capture, so a screen that is light again can clear the penalty before it +/// expires (#3844). +struct SnapshotXCTestChannelPenalty: Equatable { + static let duration: TimeInterval = 120 + static let recoveryProbeInterval: TimeInterval = 30 + + private(set) var bundleId: String? + private(set) var until = Date.distantPast + private(set) var probeDueAt = Date.distantFuture + + /// A penalty armed without a bundle id applies to whatever target is current. + func isActive(for bundleId: String?, at now: Date) -> Bool { + guard now < until else { return false } + guard let penalized = self.bundleId else { return true } + return penalized == bundleId + } + + func isProbeDue(for bundleId: String?, at now: Date) -> Bool { + isActive(for: bundleId, at: now) && now >= probeDueAt + } + + mutating func arm(bundleId: String?, at now: Date) { + self.bundleId = bundleId + until = now.addingTimeInterval(Self.duration) + probeDueAt = now.addingTimeInterval(Self.recoveryProbeInterval) + } + + /// A started probe moves the next one a full interval out, whatever the probe finds. + mutating func claimProbe(at now: Date) { + probeDueAt = now.addingTimeInterval(Self.recoveryProbeInterval) + } + + mutating func clear() { + self = SnapshotXCTestChannelPenalty() + } +} + /// How one tier's bounded work ended. `deadlineExhausted` is a tier timeout: the tier stopped /// starting work it could not finish inside its own slice, so what it returns is a partial result /// it never completed collecting. The plan keeps that result only as the fallback and lets the next @@ -64,6 +103,12 @@ struct EffectiveSnapshotCapturePlan { /// Non-nil only when the plan was narrowed by an explicit internal backend preference. This /// keeps the quality marker tied to the plan decision rather than to an untrusted request field. let preferredBackend: SnapshotBackendKind? + + /// A deferred plan that starts with an XCTest tier is running the due recovery probe. + var leadsWithXCTestRecoveryProbe: Bool { + xCTestChannelState == .deferredToIndependentBackend + && plan.first?.usesXCTestAccessibilityChannel == true + } } /// What the plan runner does when every backend failed or stayed sparse. @@ -144,10 +189,7 @@ extension RunnerTests { func penalizeSnapshotXCTestChannel(bundleId: String?, reason: String) { snapshotXCTestChannelPenaltyLock.lock() - snapshotXCTestChannelPenaltyBundleId = bundleId - let now = Date() - snapshotXCTestChannelPenaltyUntil = now.addingTimeInterval(snapshotXCTestChannelPenaltyDuration) - snapshotXCTestChannelNextRecoveryProbeAt = now.addingTimeInterval(Self.snapshotXCTestRecoveryProbeInterval) + snapshotXCTestChannelPenalty.arm(bundleId: bundleId, at: Date()) snapshotXCTestChannelPenaltyLock.unlock() NSLog( "AGENT_DEVICE_RUNNER_SNAPSHOT_XCTEST_CHANNEL_PENALIZED bundle=%@ reason=%@", @@ -158,10 +200,8 @@ extension RunnerTests { func clearSnapshotXCTestChannelPenalty(reason: String) { snapshotXCTestChannelPenaltyLock.lock() - let hadActivePenalty = Date() < snapshotXCTestChannelPenaltyUntil - snapshotXCTestChannelPenaltyBundleId = nil - snapshotXCTestChannelPenaltyUntil = Date.distantPast - snapshotXCTestChannelNextRecoveryProbeAt = Date.distantFuture + let hadActivePenalty = Date() < snapshotXCTestChannelPenalty.until + snapshotXCTestChannelPenalty.clear() snapshotXCTestChannelPenaltyLock.unlock() if hadActivePenalty { NSLog("AGENT_DEVICE_RUNNER_SNAPSHOT_XCTEST_CHANNEL_PENALTY_CLEARED reason=%@", reason) @@ -171,10 +211,20 @@ extension RunnerTests { func isSnapshotXCTestChannelPenalized(bundleId: String?) -> Bool { snapshotXCTestChannelPenaltyLock.lock() defer { snapshotXCTestChannelPenaltyLock.unlock() } - guard Date() < snapshotXCTestChannelPenaltyUntil else { return false } - // A penalty recorded without a bundle id applies to whatever target is current. - guard let penalized = snapshotXCTestChannelPenaltyBundleId else { return true } - return penalized == bundleId + return snapshotXCTestChannelPenalty.isActive(for: bundleId, at: Date()) + } + + func isSnapshotXCTestRecoveryProbeDue(bundleId: String?) -> Bool { + snapshotXCTestChannelPenaltyLock.lock() + defer { snapshotXCTestChannelPenaltyLock.unlock() } + return snapshotXCTestChannelPenalty.isProbeDue(for: bundleId, at: Date()) + } + + func claimSnapshotXCTestRecoveryProbe(bundleId: String?) { + snapshotXCTestChannelPenaltyLock.lock() + snapshotXCTestChannelPenalty.claimProbe(at: Date()) + snapshotXCTestChannelPenaltyLock.unlock() + NSLog("AGENT_DEVICE_RUNNER_SNAPSHOT_XCTEST_RECOVERY_PROBE bundle=%@", bundleId ?? "") } /// The pre-seeded first-failure a penalized plan stamps into its verdict. The deferred case @@ -247,11 +297,13 @@ extension RunnerTests { /// Pure plan-reorder rule: an internal preferred backend pins the regular plan to that backend; /// this is the only force seam used by same-backend evidence and conformance captures. A /// penalized XCTest accessibility channel uses independent backends when the platform has one, - /// otherwise it keeps XCTest work on a short probe. The raw diagnostic plan keeps tree-first - /// errors, and unknown plans are left untouched. + /// otherwise it keeps XCTest work on a short probe. A due recovery probe puts that short tree + /// ahead of the independent backend. The raw diagnostic plan keeps tree-first errors, and unknown + /// plans are left untouched. static func effectiveSnapshotCapturePlan( _ plan: [SnapshotBackendKind], xCTestChannelPenalized: Bool, + xCTestRecoveryProbeDue: Bool = false, availableBackends: Set = Set(SnapshotBackendKind.allCases), preferredBackend: String? = nil ) -> EffectiveSnapshotCapturePlan { @@ -281,10 +333,12 @@ extension RunnerTests { if !recoveryPlan.isEmpty { // The independent backend reads only the app it can match as the active AX application, so // an out-of-process surface over the app (the Save Password sheet) leaves it empty for the - // whole penalty. The tree, on the bounded probe's short slice, stays behind it; the query - // sweep does not, since its grind is what the penalty keeps off the main thread. + // whole penalty. The tree, on the bounded probe's short slice, stays behind it, or leads when + // the recovery probe is due; the query sweep does not, since its grind is what the penalty + // keeps off the main thread. + let boundedTree = availablePlan.filter { $0 == .recursiveTree } return EffectiveSnapshotCapturePlan( - plan: recoveryPlan + availablePlan.filter { $0 == .recursiveTree }, + plan: xCTestRecoveryProbeDue ? boundedTree + recoveryPlan : recoveryPlan + boundedTree, xCTestChannelState: .deferredToIndependentBackend, treeCaptureSliceBudgetOverride: Self.penalizedXCTestProbeTreeSliceBudget, preferredBackend: nil @@ -298,104 +352,6 @@ extension RunnerTests { ) } - static let snapshotXCTestRecoveryProbeInterval: TimeInterval = 30 - - static func shouldScheduleSnapshotXCTestRecoveryProbe( - effective: EffectiveSnapshotCapturePlan, - acceptedBackend: SnapshotBackendKind, - now: Date, - nextProbeAt: Date, - deadline: Date, - hasAbandonedWork: Bool - ) -> Bool { - effective.xCTestChannelState == .deferredToIndependentBackend - && effective.preferredBackend == nil - && effective.plan.contains(.recursiveTree) - && !acceptedBackend.usesXCTestAccessibilityChannel - && !hasAbandonedWork - && now >= nextProbeAt - && deadline.timeIntervalSince(now) >= Self.penalizedXCTestProbeTreeSliceBudget - } - - private func scheduleSnapshotXCTestRecoveryProbeIfNeeded( - effective: EffectiveSnapshotCapturePlan, - acceptedBackend: SnapshotBackendKind, - target: SnapshotCaptureTarget, - options: PresentationOptions, - deadline: Date - ) { - let abandoned = hasAbandonedMainThreadWork() - let now = Date() - snapshotXCTestChannelPenaltyLock.lock() - let penaltyMatches = now < snapshotXCTestChannelPenaltyUntil - && (snapshotXCTestChannelPenaltyBundleId == nil || snapshotXCTestChannelPenaltyBundleId == target.bundleId) - let scheduled = penaltyMatches && Self.shouldScheduleSnapshotXCTestRecoveryProbe( - effective: effective, - acceptedBackend: acceptedBackend, - now: now, - nextProbeAt: snapshotXCTestChannelNextRecoveryProbeAt, - deadline: deadline, - hasAbandonedWork: abandoned - ) - if scheduled { - snapshotXCTestChannelNextRecoveryProbeAt = .distantFuture - } - let penaltyUntil = snapshotXCTestChannelPenaltyUntil - snapshotXCTestChannelPenaltyLock.unlock() - guard scheduled else { return } - commandExecutionQueue.async { - self.snapshotXCTestChannelPenaltyLock.lock() - let stillCurrent = self.snapshotXCTestChannelPenaltyUntil == penaltyUntil - && Date() < penaltyUntil - if stillCurrent { - self.snapshotXCTestChannelNextRecoveryProbeAt = Date().addingTimeInterval(Self.snapshotXCTestRecoveryProbeInterval) - } - self.snapshotXCTestChannelPenaltyLock.unlock() - guard stillCurrent, !self.hasAbandonedMainThreadWork(), - deadline.timeIntervalSinceNow >= Self.penalizedXCTestProbeTreeSliceBudget - else { return } - self.runScheduledSnapshotXCTestRecoveryProbe( - target: target, options: options, channelState: effective.xCTestChannelState - ) - } - } - - private func runScheduledSnapshotXCTestRecoveryProbe( - target: SnapshotCaptureTarget, - options: PresentationOptions, - channelState: SnapshotXCTestChannelPlanState - ) { - let now = Date() - let slice = Self.penalizedXCTestProbeTreeSliceBudget - NSLog("AGENT_DEVICE_RUNNER_SNAPSHOT_XCTEST_RECOVERY_PROBE_SCHEDULED bundle=%@", target.bundleId ?? "") - defer { - NSLog("AGENT_DEVICE_RUNNER_SNAPSHOT_XCTEST_RECOVERY_PROBE_FINISHED elapsedMs=%d", Int(Date().timeIntervalSince(now) * 1_000)) - } - // One idle-only hop bounds the entire optional probe, including viewport reads and processing. - // Its abandoned work uses the ordinary occupancy gate, but never arms a new penalty. - let attempt = try? runMainThreadWorkIfIdle( - "snapshot_recovery_probe", - timeout: slice, - timeoutError: snapshotMainThreadTimeoutError("probing XCTest recovery") - ) { - try self.captureWithBackend( - .recursiveTree, - target: target, - options: options, - deadline: now.addingTimeInterval(slice), - treeCaptureSliceBudgetOverride: slice - ) - } - guard let attempt else { return } - recordXCTestSnapshotBackendAttemptIfNeeded( - .recursiveTree, - attempt: attempt, - bundleId: target.bundleId, - penaltySuppressed: false, - channelState: channelState - ) - } - func shouldSkipSnapshotBackendForAbandonedMainThreadWork(_ kind: SnapshotBackendKind) -> Bool { kind.usesXCTestAccessibilityChannel && hasAbandonedMainThreadWork() } @@ -425,8 +381,10 @@ extension RunnerTests { // deliberately, so no degradation warning should render for it. var xCTestChannelPenalized = false var xCTestChannelPenalizedByBreaker = false + var xCTestRecoveryProbeDue = false #if os(iOS) xCTestChannelPenalizedByBreaker = isSnapshotXCTestChannelPenalized(bundleId: target.bundleId) + xCTestRecoveryProbeDue = isSnapshotXCTestRecoveryProbeDue(bundleId: target.bundleId) xCTestChannelPenalized = Self.snapshotXCTestChannelTreatedAsPenalized( penalized: xCTestChannelPenalizedByBreaker, preferredBackend: options.preferredBackend @@ -435,6 +393,7 @@ extension RunnerTests { let effective = Self.effectiveSnapshotCapturePlan( plan, xCTestChannelPenalized: xCTestChannelPenalized, + xCTestRecoveryProbeDue: xCTestRecoveryProbeDue, availableBackends: Set(SnapshotBackendKind.allCases.filter(\.isAvailableOnCurrentPlatform)), preferredBackend: options.preferredBackend ) @@ -475,6 +434,9 @@ extension RunnerTests { } continue } + if kind == effectivePlan.first && effective.leadsWithXCTestRecoveryProbe { + claimSnapshotXCTestRecoveryProbe(bundleId: target.bundleId) + } if kind.usesXCTestAccessibilityChannel { xCTestTierRan = true } let attempt = try captureWithBackend( kind, @@ -536,13 +498,6 @@ extension RunnerTests { verdictReason?.reason ?? "sparse tree" ) } - scheduleSnapshotXCTestRecoveryProbeIfNeeded( - effective: effective, - acceptedBackend: kind, - target: target, - options: options, - deadline: deadline - ) return stampedSnapshotPayload( capture, backend: kind, diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotTiming.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotTiming.swift index 6f29ea4c4e..86b8454926 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotTiming.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotTiming.swift @@ -123,7 +123,6 @@ extension RunnerTests { channelState: SnapshotXCTestChannelPlanState = .normal ) -> String? { guard channelState == .normal, kind.usesXCTestAccessibilityChannel else { return nil } - let isSlow = attempt.timing.acquisitionMs > slowThresholdMs if case let .failed(failure, phase: .acquisition) = attempt.outcome, failure.code == Self.xCTestSnapshotTimeoutCode { @@ -132,7 +131,7 @@ extension RunnerTests { if attempt.tierOutcome == .deadlineExhausted { return "\(kind.rawValue)_backend_timeout" } - guard isSlow else { return nil } + guard attempt.timing.acquisitionMs > slowThresholdMs else { return nil } return "slow_\(kind.rawValue)_capture_\(Int(attempt.timing.acquisitionMs))ms" } diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift index c99fcaabe3..4c7d5516ba 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift @@ -125,10 +125,7 @@ final class RunnerTests: XCTestCase { // channel when an independent recovery backend exists, or use a bounded XCTest probe when it // does not, for the same screen class (#1105/#1156). let snapshotXCTestChannelPenaltyLock = NSLock() - var snapshotXCTestChannelPenaltyBundleId: String? - var snapshotXCTestChannelPenaltyUntil = Date.distantPast - var snapshotXCTestChannelNextRecoveryProbeAt = Date.distantFuture - let snapshotXCTestChannelPenaltyDuration: TimeInterval = 120 + var snapshotXCTestChannelPenalty = SnapshotXCTestChannelPenalty() let snapshotXCTestPenaltyWarmupExemption = SnapshotXCTestPenaltyWarmupExemption() // Sticky per-bundle hint for the private AX depth ladder: deep RN screens reject the default // depth with kAXErrorIllegalArgument on EVERY capture, so once a shallower rung is accepted diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanOccupancyTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanOccupancyTests.swift index 6388902f6e..2e2cc45490 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanOccupancyTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanOccupancyTests.swift @@ -298,8 +298,12 @@ extension RunnerTests { XCTAssertFalse(hasAbandonedMainThreadWork()) } - /// An incomplete normal-plan sweep must reach private AX and penalize the hostile channel. - func testQuerySweepDeadlineRecoversAndPenalizesChannel() throws { + /// A query sweep that stops at its own slice deadline collected a partial tree, not an answer, so + /// it is a tier timeout: the plan must arm the XCTest-channel penalty and let private AX answer, + /// keeping the partial sweep only as the fallback. Accepting it because it carries more nodes than + /// the sparse threshold ships a hierarchy-free capture and leaves every later capture of the same + /// screen to pay for the full sweep again (#2781). + func testQuerySweepThatEndsOnItsSliceDeadlinePenalizesChannelAndReachesPrivateAX() throws { guard let queryMethod = class_getInstanceMethod( XCUIElementQuery.self, @@ -341,6 +345,9 @@ extension RunnerTests { var error: Error? var abandonedAtReturn = false var penalizedAtReturn = false + var sweepStartsAfterFirstPlan = 0 + var secondPayload: DataPayload? + var secondError: Error? } let box = ResultBox() let planned = expectation(description: "slow-sweep plan answered") @@ -358,6 +365,18 @@ extension RunnerTests { } box.abandonedAtReturn = self.hasAbandonedMainThreadWork() box.penalizedAtReturn = self.isSnapshotXCTestChannelPenalized(bundleId: captureTarget.bundleId) + box.sweepStartsAfterFirstPlan = RunnerSlowSweepQueryGate.recordedStarts().count + do { + box.secondPayload = try self.runSnapshotCapturePlan( + Self.regularVisiblePlan, + target: captureTarget, + options: PresentationOptions(interactiveOnly: false, depth: nil, scope: nil, raw: false), + terminal: .sparseWithFatalOnAXFailure, + deadline: Date().addingTimeInterval(20) + ) + } catch { + box.secondError = error + } planned.fulfill() } @@ -374,7 +393,19 @@ extension RunnerTests { XCTAssertFalse(box.abandonedAtReturn, "the sweep must answer inside its own main-thread hop") XCTAssertTrue( box.penalizedAtReturn, - "a normal-plan query-sweep timeout must arm the channel penalty" + "the tier timeout must arm the XCTest-channel penalty for the captured target" + ) + + XCTAssertNil(box.secondError) + XCTAssertEqual( + box.secondPayload?.snapshotQuality?.backend, + SnapshotBackendKind.privateAX.rawValue, + "the armed penalty must defer the next plan's XCTest tiers, not re-run the sweep" + ) + XCTAssertEqual( + RunnerSlowSweepQueryGate.recordedStarts().count, + box.sweepStartsAfterFirstPlan, + "the deferred plan may start no sweep query at all" ) XCTAssertFalse(hasAbandonedMainThreadWork()) } @@ -442,12 +473,12 @@ extension RunnerTests { tearDownPenalizedPrivateAXBlindTarget() } - let originalPenaltyUntil = snapshotXCTestChannelPenaltyUntil + let originalPenaltyUntil = snapshotXCTestChannelPenalty.until let payload = try runDeferredPlanOffMain(target: captureTarget) { RunnerBlockingSnapshotGate.release.signal() } XCTAssertEqual( - snapshotXCTestChannelPenaltyUntil, + snapshotXCTestChannelPenalty.until, originalPenaltyUntil, "the bounded recovery probe must not renew its own penalty when its tree work is abandoned" ) @@ -520,9 +551,13 @@ extension RunnerTests { XCTAssertNotEqual(quality.state, .healthy, "a tree without its collapsed tab items is not healthy") } - func testScheduledRecoveryProbeAfterAcceptedPrivateAXIsBoundedAndCannotRepeatOrRearm() throws { + /// A due recovery probe runs inside the capture, ahead of private AX. Its abandoned tree work is + /// charged to that capture: the plan answers while the work is outstanding, so that response's + /// occupancy stamp reports it. The probe cannot renew the penalty, and the next plan inside the + /// interval defers without entering the tree. + func testDueRecoveryProbeRunsInsideTheCaptureAndChargesItsAbandonedWorkToIt() throws { let captureTarget = try launchPenalizedPrivateAXBlindTarget( - bundleId: "com.callstack.agentdevice.runner.scheduled-probe-test" + bundleId: "com.callstack.agentdevice.runner.due-probe-test" ) defer { tearDownPenalizedPrivateAXBlindTarget() } let options = PresentationOptions(interactiveOnly: false, depth: nil, scope: nil, raw: false) @@ -540,45 +575,38 @@ extension RunnerTests { defer { RunnerBlockingSnapshotGate.release.signal() method_setImplementation(snapshotMethod, original) - let deadline = Date().addingTimeInterval(3) - while hasAbandonedMainThreadWork(), Date() < deadline { - RunLoop.current.run(until: Date().addingTimeInterval(0.01)) - } + waitForAbandonedMainThreadWorkToDrain() } - snapshotXCTestChannelNextRecoveryProbeAt = .distantPast - let expiry = snapshotXCTestChannelPenaltyUntil - let started = Date() - let payload = try runDeferredPlanOffMain(target: captureTarget) - XCTAssertLessThan(Date().timeIntervalSince(started), 0.5, "the accepted capture must answer before the optional probe") - let probeFinished = expectation(description: "scheduled probe answered") - commandExecutionQueue.async { + snapshotXCTestChannelPenalty.claimProbe( + at: Date(timeIntervalSinceNow: -SnapshotXCTestChannelPenalty.recoveryProbeInterval) + ) + let expiry = snapshotXCTestChannelPenalty.until + final class Occupancy { var atAnswer = false } + let occupancy = Occupancy() + + let probed = try runDeferredPlanOffMain(target: captureTarget) { + occupancy.atAnswer = self.hasAbandonedMainThreadWork() RunnerBlockingSnapshotGate.release.signal() - probeFinished.fulfill() - } - wait(for: [probeFinished], timeout: 3) - XCTAssertEqual(payload.snapshotQuality?.backend, SnapshotBackendKind.privateAX.rawValue) - XCTAssertEqual(RunnerBlockingSnapshotGate.entered.wait(timeout: .now()), .success) - XCTAssertLessThan(Date().timeIntervalSince(started), 2) - XCTAssertEqual(snapshotXCTestChannelPenaltyUntil, expiry) - let next = snapshotXCTestChannelNextRecoveryProbeAt - XCTAssertGreaterThan(next.timeIntervalSince(started), 29) - _ = try runDeferredPlanOffMain(target: captureTarget) - let queueDrained = expectation(description: "second capture queued no probe") - commandExecutionQueue.async { queueDrained.fulfill() } - wait(for: [queueDrained], timeout: 3) - XCTAssertEqual(snapshotXCTestChannelNextRecoveryProbeAt, next) - XCTAssertEqual(snapshotXCTestChannelPenaltyUntil, expiry) - XCTAssertEqual(RunnerBlockingSnapshotGate.entered.wait(timeout: .now()), .timedOut) - - snapshotXCTestChannelNextRecoveryProbeAt = .distantPast - _ = try runDeferredPlanOffMain(target: captureTarget) { - self.clearSnapshotXCTestChannelPenalty(reason: "replace-before-probe") - self.penalizeSnapshotXCTestChannel(bundleId: captureTarget.bundleId, reason: "new-penalty") - } - let staleProbeDrained = expectation(description: "stale queued probe refused") - commandExecutionQueue.async { staleProbeDrained.fulfill() } - wait(for: [staleProbeDrained], timeout: 3) - XCTAssertEqual(RunnerBlockingSnapshotGate.entered.wait(timeout: .now()), .timedOut) + } + + XCTAssertEqual(RunnerBlockingSnapshotGate.entered.wait(timeout: .now()), .success, "the probe ran before the answer") + XCTAssertTrue(occupancy.atAnswer, "the capture that ran the probe answers as busy") + XCTAssertEqual(probed.snapshotQuality?.backend, SnapshotBackendKind.privateAX.rawValue) + XCTAssertEqual(probed.snapshotQuality?.reasonCode, "deferred") + XCTAssertEqual(snapshotXCTestChannelPenalty.until, expiry, "a failed probe must not renew the penalty") + XCTAssertFalse(isSnapshotXCTestRecoveryProbeDue(bundleId: captureTarget.bundleId)) + waitForAbandonedMainThreadWorkToDrain() + + let deferred = try runDeferredPlanOffMain(target: captureTarget) + XCTAssertEqual(deferred.snapshotQuality?.backend, SnapshotBackendKind.privateAX.rawValue) + XCTAssertEqual(RunnerBlockingSnapshotGate.entered.wait(timeout: .now()), .timedOut, "no probe inside the interval") + } + + private func waitForAbandonedMainThreadWorkToDrain() { + let deadline = Date().addingTimeInterval(3) + while hasAbandonedMainThreadWork(), Date() < deadline { + RunLoop.current.run(until: Date().addingTimeInterval(0.01)) + } } private func launchPenalizedPrivateAXBlindTarget(bundleId: String) throws -> SnapshotCaptureTarget { @@ -620,7 +648,7 @@ extension RunnerTests { } let box = ResultBox() let planned = expectation(description: "deferred plan answered") - commandExecutionQueue.async { + DispatchQueue(label: "agent-device.runner.tests.deferred-plan").async { do { box.payload = try self.runSnapshotCapturePlan( Self.regularVisiblePlan, diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanTests.swift index 045ca2d6aa..4d88f3c9d7 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanTests.swift @@ -538,10 +538,7 @@ extension RunnerTests { } func testSnapshotXCTestChannelPenaltyMatchesBundleAndExpires() { - defer { - snapshotXCTestChannelPenaltyBundleId = nil - snapshotXCTestChannelPenaltyUntil = .distantPast - } + defer { snapshotXCTestChannelPenalty.clear() } penalizeSnapshotXCTestChannel(bundleId: "xyz.blueskyweb.app", reason: "test") XCTAssertTrue(isSnapshotXCTestChannelPenalized(bundleId: "xyz.blueskyweb.app")) @@ -552,7 +549,9 @@ extension RunnerTests { XCTAssertTrue(isSnapshotXCTestChannelPenalized(bundleId: "com.other.app")) // Expired penalties stop applying. - snapshotXCTestChannelPenaltyUntil = Date(timeIntervalSinceNow: -1) + snapshotXCTestChannelPenalty.arm( + bundleId: nil, at: Date(timeIntervalSinceNow: -SnapshotXCTestChannelPenalty.duration - 1) + ) XCTAssertFalse(isSnapshotXCTestChannelPenalized(bundleId: "com.other.app")) } @@ -655,42 +654,70 @@ extension RunnerTests { kind: .recursiveTree, attempt: fast, channelState: .normal )) } - func testScheduledRecoveryProbeRequiresDueIndependentRegularPlanAndIdleBudget() { - let now = Date(timeIntervalSinceReferenceDate: 100) - let deferred = Self.effectiveSnapshotCapturePlan(Self.regularVisiblePlan, xCTestChannelPenalized: true) - func schedules( - _ effective: EffectiveSnapshotCapturePlan, - backend: SnapshotBackendKind = .privateAX, - next: Date = now, - remaining: TimeInterval = 1, - abandoned: Bool = false - ) -> Bool { - Self.shouldScheduleSnapshotXCTestRecoveryProbe( - effective: effective, acceptedBackend: backend, now: now, nextProbeAt: next, - deadline: now.addingTimeInterval(remaining), hasAbandonedWork: abandoned + + func testDueRecoveryProbeLeadsOnlyTheDeferredRegularPlanAndNeverAddsTheSweep() { + let due = Self.effectiveSnapshotCapturePlan( + Self.regularVisiblePlan, xCTestChannelPenalized: true, xCTestRecoveryProbeDue: true + ) + XCTAssertEqual(due.plan, [.recursiveTree, .privateAX]) + XCTAssertEqual(due.xCTestChannelState, .deferredToIndependentBackend) + XCTAssertEqual(due.treeCaptureSliceBudgetOverride, Self.penalizedXCTestProbeTreeSliceBudget) + XCTAssertTrue(due.leadsWithXCTestRecoveryProbe) + + let notDue = Self.effectiveSnapshotCapturePlan(Self.regularVisiblePlan, xCTestChannelPenalized: true) + XCTAssertEqual(notDue.plan, [.privateAX, .recursiveTree]) + XCTAssertFalse(notDue.leadsWithXCTestRecoveryProbe) + + let physicalDevice = Self.effectiveSnapshotCapturePlan( + Self.regularVisiblePlan, xCTestChannelPenalized: true, xCTestRecoveryProbeDue: true, + availableBackends: [.recursiveTree, .querySweep] + ) + XCTAssertEqual(physicalDevice.plan, [.recursiveTree, .querySweep]) + XCTAssertFalse(physicalDevice.leadsWithXCTestRecoveryProbe, "the bounded plan probes on every capture") + for pinned in ["private-ax", "tree"] { + let plan = Self.effectiveSnapshotCapturePlan( + Self.regularVisiblePlan, xCTestChannelPenalized: true, xCTestRecoveryProbeDue: true, + preferredBackend: pinned ) + XCTAssertEqual(plan.plan.count, 1) + XCTAssertFalse(plan.leadsWithXCTestRecoveryProbe) } - XCTAssertTrue(schedules(deferred)) - XCTAssertFalse(schedules(deferred, next: now.addingTimeInterval(0.001))) - XCTAssertFalse(schedules(deferred, remaining: 0.999)) - XCTAssertFalse(schedules(deferred, abandoned: true)) - XCTAssertFalse(schedules(deferred, backend: .recursiveTree)) - XCTAssertFalse(schedules(deferred, backend: .querySweep)) - XCTAssertFalse(schedules(Self.effectiveSnapshotCapturePlan( - Self.regularVisiblePlan, xCTestChannelPenalized: false - ))) - XCTAssertFalse(schedules(Self.effectiveSnapshotCapturePlan( - Self.rawDiagnosticPlan, xCTestChannelPenalized: true - ))) - XCTAssertFalse(schedules(Self.effectiveSnapshotCapturePlan( - Self.regularVisiblePlan, xCTestChannelPenalized: true, - availableBackends: [.recursiveTree, .querySweep] - ))) - XCTAssertFalse(schedules(Self.effectiveSnapshotCapturePlan( - Self.regularVisiblePlan, xCTestChannelPenalized: true, preferredBackend: "private-ax" - ))) - XCTAssertEqual(Self.snapshotXCTestRecoveryProbeInterval, 30) + let raw = Self.effectiveSnapshotCapturePlan( + Self.rawDiagnosticPlan, xCTestChannelPenalized: true, xCTestRecoveryProbeDue: true + ) + XCTAssertEqual(raw.plan, Self.rawDiagnosticPlan) + XCTAssertFalse(raw.leadsWithXCTestRecoveryProbe) } + func testChannelPenaltyProbeIsDueEachIntervalUntilItExpiresOrClears() { + let armedAt = Date(timeIntervalSinceReferenceDate: 1_000) + var penalty = SnapshotXCTestChannelPenalty() + XCTAssertFalse(penalty.isActive(for: "app", at: armedAt)) + XCTAssertFalse(penalty.isProbeDue(for: "app", at: armedAt)) + + penalty.arm(bundleId: "app", at: armedAt) + XCTAssertTrue(penalty.isActive(for: "app", at: armedAt)) + XCTAssertFalse(penalty.isActive(for: "other", at: armedAt)) + XCTAssertFalse(penalty.isProbeDue(for: "app", at: armedAt.addingTimeInterval(29.9))) + XCTAssertTrue(penalty.isProbeDue(for: "app", at: armedAt.addingTimeInterval(30))) + XCTAssertFalse(penalty.isProbeDue(for: "other", at: armedAt.addingTimeInterval(30))) + + let probedAt = armedAt.addingTimeInterval(45) + penalty.claimProbe(at: probedAt) + XCTAssertTrue(penalty.isActive(for: "app", at: probedAt)) + XCTAssertFalse(penalty.isProbeDue(for: "app", at: probedAt.addingTimeInterval(29.9))) + XCTAssertTrue(penalty.isProbeDue(for: "app", at: probedAt.addingTimeInterval(30))) + XCTAssertFalse( + penalty.isProbeDue(for: "app", at: armedAt.addingTimeInterval(SnapshotXCTestChannelPenalty.duration)), + "an expired penalty has no probe to run" + ) + + penalty.clear() + XCTAssertFalse(penalty.isActive(for: "app", at: probedAt)) + XCTAssertFalse(penalty.isProbeDue(for: "app", at: .distantFuture)) + + penalty.arm(bundleId: nil, at: armedAt) + XCTAssertTrue(penalty.isActive(for: "any", at: armedAt), "a bundle-less penalty applies to any target") + } } #endif diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotTimingTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotTimingTests.swift index 82e8d238f4..9a5033934b 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotTimingTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotTimingTests.swift @@ -60,7 +60,10 @@ extension RunnerTests { ) } - func testDeadlineExhaustedTreeAttemptArmsTheChannelPenaltyWithoutBeingSlow() { + /// A tier that stopped starting work at its own deadline is a timeout for the breaker even when it + /// answered fast and with a payload: the partial sweep must arm the penalty exactly as a slow tier + /// does, and a tier that finished fast must not (#2781). + func testDeadlineExhaustedTierAttemptArmsTheChannelPenaltyWithoutBeingSlow() { let capture = SnapshotBackendCapture( payload: DataPayload(nodes: [], truncated: true), effectiveDepth: nil @@ -72,11 +75,11 @@ extension RunnerTests { ) XCTAssertEqual( Self.snapshotXCTestPenaltyReason( - kind: .recursiveTree, + kind: .querySweep, attempt: exhausted, slowThresholdMs: 3_000 ), - "tree_backend_timeout" + "queries_backend_timeout" ) let completed = SnapshotBackendAttempt( From 7640bdc6e6e7554b842f08366facb5ea6c574f0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 22 Oct 2026 12:17:02 +0200 Subject: [PATCH 5/5] docs(adr): record the in-band XCTest recovery probe in ADR 0004 Replace the post-response probe text with the in-band policy and note that, after the iPad amendment, Simulator captures meet it only when the bridge sends them to XCTest. Refs #3844 Co-Authored-By: Claude Opus 5.5 --- docs/adr/0004-ios-snapshot-backend-strategy.md | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/docs/adr/0004-ios-snapshot-backend-strategy.md b/docs/adr/0004-ios-snapshot-backend-strategy.md index 0ba88b0ffe..0d7d1c78c3 100644 --- a/docs/adr/0004-ios-snapshot-backend-strategy.md +++ b/docs/adr/0004-ios-snapshot-backend-strategy.md @@ -245,13 +245,12 @@ Issue #3844 showed that the bounded recovery probe could renew its own penalty. A bounded recovery probe now leaves the penalty deadline unchanged when it fails. Its short slice tests recovery under a known penalty, so its timeout supplies no new evidence about the channel. A completed tree probe clears the penalty early only when its acquisition fits the probe slice and its quality is accepted. -After an independent backend succeeds, a penalized regular plan can schedule the existing bounded tree probe once per 30 seconds. -The interval starts when the penalty is armed. -The one-second idle-only probe bounds all acquisition and processing work. -The accepted independent capture remains the response. -The existing serial command queue runs the optional probe after that response. -A queued probe reserves the interval and checks that its penalty is still current before it starts. -The schedule excludes physical-device plans, explicit backend pins, occupied main-thread work, and plans with less than one second remaining. +Thirty seconds after the penalty is armed, and thirty seconds after each probe starts, a recovery probe is due. +A regular plan with a due probe runs the bounded tree before the independent backend, in the same capture. +The runner starts no XCTest main-thread work after it answers a command. +Probe work that outlives its slice is charged to the capture that started it, so that response reports the main thread as busy. +Physical-device plans probe on every capture already. Explicit backend pins and the raw plan keep their order. +After the iPad amendment below, a Simulator capture meets this policy only when the bridge sends it to XCTest, for example for a windowed app or a presented system surface. The interval limits hostile-screen probe cost while giving healthy screens a recovery opportunity before penalty expiry. A normal-plan query-sweep timeout rejects the partial capture and penalizes the channel. Its short slice is evidence of a hostile tree, and deferral prevents repeated slow sweeps on that tree.