fix(ios): keep the runner alive through hostile snapshots

[okwasniewski]

Stacked on #2630 (pure move of the main-thread work machinery). Review this PR from its second commit.

Summary

On a live React Native feed (Bluesky Home) the iOS runner died after every snapshot: each next command paid an xcodebuild boot.

Cause: the query-sweep tier records 19 XCTest issues per capture worded Failed to resolve query: … kAXErrorIllegalArgument …. record(_:) only muted the sibling Failed to get matching snapshot wording, and XCTest ends testCommand once the main-thread block that recorded an unmuted failure returns.

Changes, on top of the move (7 files modified, 1 added):

• Mute AX-server rejections in both fetch wordings; timeouts and Application X is not running keep recording.
• One abandoned-main-thread-work counter: runMainThreadWork takes an operation label and logs abandon/drain. An abandoned viewport read or tree XPC now skips the query sweep: first hostile capture ~2.5 s instead of ~4.5 s. The tree slice timeout penalizes through its attempt outcome, so the warmup exemption applies.
• Separate commit: a failed capture queues its cached-target drop behind abandoned work instead of waiting a second for it.
• Docs: CHANGELOG, ADR 0004.

Validation

Tested commit ab34abc27 (rebased onto 465af7549). Earlier evidence at 7e2b10caf.

• iOS runner XCTest suite: 243 tests, 0 failures. Both new tests fail on a copy with the fix reverted: 2 of 2 fail (4 assertions).
• Live Bluesky feed through the runner path: 1 runner boot for 58 commands, 0 recorded XCTest failures. Before: one boot per hostile snapshot.
• Local gates pass: format, lint, typecheck, xctest-selection, packaged-runner-swift, gate-manifest. pnpm check:affected --run: every runnable check passes except mutation-model (ownership.test.ts, "a kernel is owned by tests that reach it indirectly"), pre-existing on main: this branch changes no TypeScript.
• macOS host lane: CI only (unsigned bundle refused locally).

Conflict-resolution validation

Rebased onto the updated #2630 extraction; new head 0bdf5a04a5. All three behavior/gate commits are unchanged by git range-diff. iOS and macOS build-for-testing passed with unit tests enabled. pnpm check:affected --run stopped at the pre-existing mutation-model assertion “a kernel is owned by tests that reach it indirectly” (scripts/mutation/ownership.test.ts:40); the same failure was reproduced on unchanged main 465af7549b. Earlier device evidence remains attributed to the earlier head; no new device run was performed. New-head GitHub CI is pending.

[Copilot]

🔵 Needs a closer look

The regression test does not exercise a genuinely blocked main-thread operation, so deferred invalidation is not fully verified.

Pull request overview

Improves iOS runner resilience during hostile accessibility snapshots by suppressing known AX failures and tracking abandoned main-thread work.

Changes:

• Adds deferred invalidation and abandoned-work tracking.
• Expands AX failure suppression and snapshot recovery.
• Adds tests, documentation, changelog, and CI coverage.

File summaries

File	Description
docs/adr/0004-ios-snapshot-backend-strategy.md	Documents snapshot recovery strategy.
CHANGELOG.md	Records the runner-liveness fix.
apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+RecordedIssueSuppressionTests.swift	Tests AX issue classification.
apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift	Skips XCTest tiers during abandoned work.
apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Snapshot.swift	Applies centralized work tracking.
apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+MainThreadWork.swift	Adds main-thread work infrastructure.
apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift	Defers invalidation and updates command handling.
apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+AXSnapshotFallback.swift	Avoids blocked viewport reads.
apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift	Expands AX suppression behavior.
.github/workflows/ios.yml	Selects the new iOS regression test.

Review details

Suppressed comments (1)

apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift:853

• This regression test does not actually hold a main-thread operation behind the abandoned count; it only sets the counter manually. With the previous implementation, runMainThreadWork would dispatch invalidation to a free main queue and complete before the 0.5 s assertion too, so this test would also pass after reverting the deferred branch. Please block a real runMainThreadWork operation, assert the target remains cached while it is blocked, then release it and verify the queued invalidation runs.

  func testSnapshotFailureInvalidationQueuesBehindAbandonedMainThreadWorkWithoutWaiting() {
    currentBundleId = "com.example.stale-target"
    abandonedMainThreadWorkCount = 1
    defer {
      abandonedMainThreadWorkCount = 0

• Files reviewed: 10/10 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[thymikee]

Reviewed at 9844aa8. The classifier fix for Failed to resolve query looks right, and the single abandoned-work counter is a good cleanup. The two new regression tests do not guard the behavior they describe, though.

testSnapshotFailureInvalidationQueuesBehindAbandonedMainThreadWorkWithoutWaiting (RunnerTests+CommandExecution.swift#L849) sets abandonedMainThreadWorkCount = 1 by hand while the main thread is free. wait(for:) services the main queue, so if you delete the new hasAbandonedMainThreadWork() guard, the old runMainThreadWork(.targetInvalidation, timeout: 1) hop finishes in milliseconds and the test stays green. A revert of the deferral would pass CI. Can the test hold the main thread with real work instead? For example: start a runMainThreadWork(..., timeout: 0) block that waits on a semaphore, call invalidateCachedTargetAfterSnapshotFailure off-main, assert it returns in under 0.5 s while currentBundleId is still set, then release the semaphore and assert the target drop runs and the count drains to 0.

testAbandonedSnapshotViewportReadOccupiesMainThreadUntilDrained (RunnerTests+Snapshot.swift#L867) calls makeSnapshotTraversalContext directly and checks the counter and busy state, which runMainThreadWork already tracked before this PR. It never runs the capture plan, so two changes have no test: the query sweep is skipped after an abandoned viewport or tree read, and a tree slice timeout now honors the warmup exemption. Moving the penalizeSnapshotXCTestChannel call back into onAbandoned keeps every test green. Could a plan-level test drive the visible plan with the existing blocking frame stub, assert the payload comes from privateAX with no querySweep attempt, and assert the channel is not penalized while the warmup exemption is pending?

A design question: the deferred invalidation and the 330-line move into RunnerTests+MainThreadWork.swift read as separate changes. Would it be simpler to ship them apart? And does MainThreadOperation do more than label a log line, or would a string do? Production growth is within the size guideline, so this is only a question.

One more question on scope: after an abandoned viewport or tree slice, a slow but healthy screen now also gets privateAX output with no sweep. Did the Bluesky run, or any other screen, show that output is still complete enough?

Thanks for the live Bluesky numbers (1 runner boot for 58 commands on 9844aa8). A short runner log excerpt from that run, showing the suppressed Failed to resolve query issues and the querySweep skip after an abandoned viewport read, would make the result easy to check.

CI is green, and the iOS lane now runs the new viewport test. There are no merge conflicts. Next step: make the two tests fail when the new behavior is reverted.

[okwasniewski]

Thanks for the close read. Both test points were right, and the stack is restructured as you suggested.

Restructured into a stack. The pure move now lives in its own PR (#2630, refactor(move)), and this PR sits on top of it. Within this PR the deferred target drop is its own commit (c9faeca4b) after the fix commit (d03544469), and the ios.yml lane entry stays in chore(gates).

MainThreadOperation is gone. It only labelled the log line, so runMainThreadWork now takes a plain string label.

Tests now fail on revert.

• testSnapshotFailureInvalidationQueuesBehindAbandonedMainThreadWorkWithoutWaiting holds the main thread with a real runMainThreadWork(timeout: 0) block on a semaphore, calls the invalidation off-main, asserts it returns in under 0.5 s while currentBundleId is still set and the abandoned count is still 1, then releases and asserts the drop ran and the count drained.
• testAbandonedTreeCaptureSkipsQuerySweepAndHonorsWarmupExemption (new, iOS lane) swizzles -[XCUIElement snapshotWithError:] to block, drives runSnapshotCapturePlan on the host app with the warmup exemption pending, and asserts: payload from private AX marked recovered with the tree-slice reason, exactly one abandoned unit (a queued sweep makes it 2), and no channel penalty. The old viewport-read test is deleted.
• On a copy with the guard removed, the penalty moved back into onAbandoned, and the sweep skip disabled: both fail (4 assertions: abandoned count 2 instead of 1, penalized despite the exemption, a 1.02 s wait, and the phantom abandoned unit). On the branch: 243 tests, 0 failures at 7e2b10caf.

Completeness after an abandoned read. In the session log that prompted this (1315 commands), the query sweep recovered a capture once out of 730 recoveries and timed out 344 times, always after the tree or viewport read ahead of it had already ground. The sweep is a main-thread dispatch, so once main is occupied it cannot start before its own slice expires; skipping it changes what is logged, not what is returned. The live run's first hostile capture returned 207 private-AX nodes at depth 56, the same shape the original log shows for the recoveries that survived.

Runner log excerpt from the live Bluesky run on the fix (first hostile capture, bridge disabled so the runner served it):

20:32:13.299 AGENT_DEVICE_RUNNER_MAIN_THREAD_WORK_ABANDONED operation=snapshot_viewport slice=1.0
20:32:13.315 AGENT_DEVICE_RUNNER_SNAPSHOT_XCTEST_CHANNEL_PENALIZED bundle=xyz.blueskyweb.app reason=tree_backend_timeout
20:32:13.319 AGENT_DEVICE_RUNNER_SNAPSHOT_BACKEND_FAILED backend=tree error=timed out while preparing tree snapshot on the XCTest main thread
20:32:13.321 AGENT_DEVICE_RUNNER_SNAPSHOT_TIER_SKIPPED_XCTEST_OCCUPIED tier=queries
20:32:13.644 AGENT_DEVICE_RUNNER_PRIVATE_AX_SNAPSHOT_DEPTH_RETRY depth=64 error=Error kAXErrorIllegalArgument getting snapshot for element
20:32:14.442 AGENT_DEVICE_RUNNER_AX_SNAPSHOT_ISSUE_SUPPRESSED count=1 description=Failed to get matching snapshot: Error kAXErrorIllegalArgument getting snapshot for element
20:32:14.442 AGENT_DEVICE_RUNNER_MAIN_THREAD_WORK_DRAINED operation=snapshot_viewport
20:32:14.442 AGENT_DEVICE_RUNNER_ABANDONED_WORK_DRAINED
20:32:14.806 AGENT_DEVICE_RUNNER_PRIVATE_AX_DEPTH_REMEMBERED depth=56 bundle=xyz.blueskyweb.app
20:32:14.808 AGENT_DEVICE_RUNNER_PRIVATE_AX_SNAPSHOT_USED nodes=207 depth=56 extended=32

The Failed to resolve query issues no longer appear because the sweep that produced them no longer runs on that path; the suppression is exercised by the tree read's own Failed to get matching snapshot issue above, and by the classifier test for the sweep wording. Across that run: 1 runner boot for 58 commands, 0 recorded XCTest failures.

[Copilot]

🔵 Needs a closer look

Broad XCTest lifecycle and main-thread coordination changes warrant final human review; one comment nit also remains.

Review details

Suppressed comments (1)

apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+RecordedIssueSuppressionTests.swift:51

• This comment is now inaccurate: the classifier deliberately covers both the matching-snapshot and query-resolution fetches, so describing the negative case as only being outside the matching-snapshot context no longer matches the behavior. Please update it to refer to either supported fetch context.

    // A kAXError outside the matching-snapshot fetch context is not this class.

• Files reviewed: 11/11 changed files
• Comments generated: 0 new
• Review effort level: Lite

[thymikee]

Reviewed 7e2b10cafa0b6502416fcb8b768c91fd348d59ed, with the behavior changes compared against the #2630 extraction (169fdc2c). No remaining code findings; good to go once the known conflicts are resolved and the resulting head is green.

The earlier review gaps are addressed:

• One abandoned-work counter now governs runner busy state, XCTest-backed tier admission, and post-capture bookkeeping. Private AX remains available while main is blocked.
• Tree timeout penalties flow through the existing attempt policy, preserving the warmup exemption. Deferred target invalidation queues the actual cleanup without adding another timed-out wait.
• The regressions now block the real main thread and exercise the capture plan. The reported revert run fails both regressions; the fixed head passes 243 iOS tests.
• Issue suppression remains limited to AX-server rejection in the two fetch wordings; timeout and app-not-running issues still record.

Current CI: 11 successful checks, 2 skipped, none failing or pending. The PR's live Bluesky evidence reports one runner boot across 58 commands, zero recorded XCTest failures, and a recovered 207-node snapshot. The new query-resolution wording is unit-covered; the live run avoids that sweep through the occupancy fix.

This was a source/evidence review; I did not rerun device tests locally. GitHub still reports the acknowledged merge conflicts.

[thymikee]

Rebased onto the updated #2630 extraction; new head 0bdf5a04a5. All three behavior/gate commits are unchanged by git range-diff. iOS and macOS build-for-testing passed with unit tests enabled. pnpm check:affected --run stopped at the pre-existing mutation-model assertion “a kernel is owned by tests that reach it indirectly” (scripts/mutation/ownership.test.ts:40); the same failure was reproduced on unchanged main 465af7549b. Earlier device evidence remains attributed to the earlier head; no new device run was performed.

Conflict resolution pushed; marked ready-for-human. New-head CI must finish before merge.

[Copilot]

🔵 Needs a closer look

The iOS runner and XCTest lifecycle changes require final human review.

Review details

• Files reviewed: 11/11 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

A critical admission-ordering race can accept a subsequent command before deferred invalidation completes.

Review details

• Files reviewed: 11/11 changed files
• Comments generated: 1
• Review effort level: Lite

[okwasniewski]

Thanks for landing this. Two notes for the record:

• My 13:34 force-push replaced your 13:22 rebase of both branches with my own rebase of the same commits. The trees were byte-identical, so nothing changed in what merged, but it cost you a CI rerun and I should have checked the remote head when the lease refused. Sorry for the churn.
• Follow-up in test(ios): resolve the host app before stubbing its tree snapshot #2633: on a freshly booted simulator, with testAbandonedTreeCaptureSkipsQuerySweepAndHonorsWarmupExemption as the first test to launch the host app, the runner was killed with SIGTERM about 30 s in. The test now resolves the application element before installing the stub and gives the plan wait more headroom; verified with three cold-boot runs and a fresh-boot full suite (248 tests, 0 failures).