fix(ios-runner): keep snapshot-plan target state on main and bound the query sweep by its slice by thymikee · Pull Request #2836 · callstack/agent-device

thymikee · 2026-09-23T16:20:32Z

Summary

Two related iOS runner snapshot-plan bugs:

fix(ios-runner): snapshot capture plan reads and writes main-owned target state off the main thread #2781: on a blocked/abandoned main thread, snapshotAccessibilityUnavailable wrote bundle
identity and accessibility health directly instead of keeping snapshot-plan target state on
main, risking state for the wrong target. Fix: take target identity on main and hop plan-state
writes behind abandoned main-thread work. The coordinate tap's system-modal routing probe now
reads currentBundleId only on main too.
fix(ios-runner): non-interactive query sweep outlives its 1s tier slice and causes RUNNER_BUSY #2783: the query-sweep tier admitted queries against the whole plan deadline instead of its own
slice deadline, letting a slow query starve later tiers. A sweep that ends on its slice deadline
is now a typed tier timeout (SnapshotTierOutcome.deadlineExhausted), so it arms the XCTest-channel
penalty and falls through to private AX — the plan branches on the tier outcome, not node count.

Closes #2781, Closes #2783.

Validation

Rebased onto origin/main (fd8277e391); head 7414149edb. main had moved the runner's inline unit
tests into UnitTests/ (#2854) and split RunnerTests+CommandExecution.swift along command families
(#2855), so the conflicts were file-location/file-split, not semantic. Adopted main's layout: the
snapshot-command production changes live in RunnerTests+SnapshotExecution.swift, the routing probe in
RunnerTests+CommandDispatch.swift, and this PR's tests in the UnitTests/…Tests.swift file main now
owns for each family (final commit test(ios): place this PR's runner tests in main's split UnitTests files). No RunnerTests+CommandExecution.swift content was resurrected and no test main carries was
duplicated.

Both re-opened findings were re-proven on the rebased sources on an iPhone 17 / iOS 26.2 simulator
(created for the run, deleted after); the runner TEST BUILD SUCCEEDED with
AGENT_DEVICE_RUNNER_UNIT_TESTS and both pin tests passed, so main's refactor broke neither — no
test-first red-fix was required:

query-sweep tier timeout (fix: skill should work, even if the npm package is not installed #1): testQuerySweepThatEndsOnItsSliceDeadlinePenalizesChannelAndReachesPrivateAX
— passed (4.7s). Asserts the armed queries_backend_timeout penalty, the next plan deferring its
XCTest tiers, and private-ax answering.
tap-routing identity (Update README with correct GitHub link #2): testCoordinateTapRoutingPenalizesTheIdentityMainSettledOnWhileTheWriteWasPending
— passed (7.9s). Asserts the abandoned routing probe penalizes the identity main settled on, not the
one whose write was still pending.

The 9 relocated unit tests over the moved code also pass (query-sweep slice helpers, the tier-rejection
pair, the fail-closed invalidation-on-main hop, the snapshotFast/snapshotRaw modal-probe route, the
viewport bundle-id test): Executed 9 tests, with 0 failures.

Gates at 7414149edb: pnpm format clean, pnpm lint and pnpm typecheck pass,
pnpm check:xctest-selection clean (295 declared AgentDeviceRunnerUITests methods, 0 reachable by no
lane), pnpm check:affected --run all runnable checks passed.

The requested live triple (tree tier fails fast while the sweep is slow, then
reason=queries_backend_timeout, then private-ax) is not reachable through the CLI on a stock
surface: the tree tier answers first and the runner has no fault-injection seam for a real
kAXErrorIllegalArgument refusal or 50 000 presented nodes. The on-simulator XCTest above is the
equivalent proof — it plants the fast tree failure and a gated slow sweep and asserts the full
penalize → defer → private-ax sequence.

github-actions · 2026-09-23T16:25:57Z

Size Report

Metric	Base	Current	Diff
Installed (including dependencies)	4.79 MB	4.79 MB	+5.7 kB
Package (unpacked)	4.79 MB	4.79 MB	+5.7 kB
Package (download)	1.43 MB	1.43 MB	+1.4 kB

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	27.2 ms	27.4 ms	+0.2 ms
CLI --help	80.0 ms	79.7 ms	-0.2 ms

thymikee · 2026-09-23T16:41:50Z

Reviewed at 8f69c76.

A non-interactive snapshot where the tree tier fails fast and the query sweep runs long now returns a shallow partial result instead of arming the timeout penalty. runFlatInteractiveQueries stops at the slice deadline, and sparsePayloadReason accepts anything over 8 nodes (https://github.com/callstack/agent-device/blob/8f69c76/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Snapshot.swift#L462), so the plan marks the flat result recovered and never falls through to privateAX. The user loses the hierarchy, and every later snapshot on that screen pays the sweep cost again. Should a sweep that ends on its slice deadline behave like the timeout it replaces: arm the penalty and continue to privateAX, keeping the partial sweep as the fallback? A plan-level test with slow queries that asserts privateAX answers and the penalty is armed would prove it.

shouldRouteToSpringboardBlockingSystemModal now reads currentBundleId on commandExecutionQueue, in the off-main branch (https://github.com/callstack/agent-device/blob/8f69c76/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift#L2657). applyMainOwnedSnapshotState writes that state on DispatchQueue.main.async, so a coordinate tap can read it while main is still clearing the stored target. This is the same kind of race the issue wants to remove. Can this read move into the main-side closure, with the other executeDispatched callers that read target identity before the main hop checked too?

Not blocking: the 0.1 s admission margin in querySweepCanStartQuery is fixed, so the fake-clock test passes at 0.475 s query length but fails at 0.44 s; SnapshotXCTestPenaltyWarmupExemption adds its own lock instead of using snapshotXCTestChannelPenaltyLock as the issue asked; and each off-main capture adds a synchronous main hop for health, which could turn a slow hop into RUNNER_BUSY.

On size: the diff adds about 490 net lines, with four new seams. Could SnapshotXCTestPenaltyWarmupExemption be a consume function under the existing lock, and SnapshotCommandPreparation a field on the existing prepared-context type, leaving SnapshotCaptureTarget and applyMainOwnedSnapshotState as the two seams with real logic? If not, what rules that out?

No run shows the query-sweep tier inside the full regularVisiblePlan. Please add a simulator run of a plan where the tree tier fails fast and the sweep is slow, showing snapshotQuality.backend, whether AGENT_DEVICE_RUNNER_SNAPSHOT_XCTEST_CHANNEL_PENALIZED appears in runner.log, and that the next snapshot goes to privateAX.

Checks were still queued at review time. This diff overlaps the iOS Smoke route and the Repo Guards XCTest list, so failures there need a look. The known live-snapshot-depth-frontier.ts:126 failure also fails on main and #2832 fixes it.

The next step is the two fixes above, each with a plan-level test.

thymikee · 2026-09-23T17:37:38Z

Reviewed at 9719773. Both blocking findings from the earlier review (8f69c76, #2836 (comment)) are still open; this is a rebase, and commits 1, 2, and 4 are byte-identical in range-diff, so neither fix landed yet.

In a non-interactive plan the tree tier fails fast and the query sweep runs long. querySweepCanStartQuery / runFlatInteractiveQueries stop at the slice deadline (https://github.com/callstack/agent-device/blob/9719773/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Snapshot.swift#L146), and sparsePayloadReason accepts any result over 8 nodes, so the plan marks the partial flat sweep recovered without arming the XCTest-channel penalty and without falling through to privateAX. The user gets a shallow, hierarchy-free snapshot instead of a privateAX capture, and every later snapshot on that screen pays the full sweep cost again because the penalty that the old path armed never gets armed. A sweep that ends on its slice deadline must count as a tier timeout: it must arm the penalty and continue to privateAX, keeping the partial sweep only as a fallback. runFlatInteractiveQueries should return a typed outcome (completed vs. deadlineExhausted) and the capture plan should branch on that, not on node count. Please add a plan-level test with slow fake queries that asserts privateAX answers and the penalty is armed, and confirm it fails on the current head before the fix.

shouldRouteToSpringboardBlockingSystemModal still reads currentBundleId on commandExecutionQueue in the off-main branch (https://github.com/callstack/agent-device/blob/9719773/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift#L2657), while applyMainOwnedSnapshotState writes the same state through DispatchQueue.main.async. A coordinate tap can read a target that main is still clearing or rebinding, and the tap can end up routed to SpringBoard or kept on the app based on stale identity — the same cross-thread race #2781 asks this PR to remove. Target identity (currentBundleId, currentAppProcessIdentifier, and anything derived from them) should be read and written only on main, so the routing decision belongs inside the main-side closure. Every executeDispatched caller that reads target identity before its main hop should be enumerated by grepping for currentBundleId/currentAppProcessIdentifier reads outside main-hopped closures, and each one fixed. Please add a test that interleaves a pending main-side clear with an off-main tap.

No simulator run yet shows the query-sweep tier inside a full regularVisiblePlan, so there's no live proof of how the backend picks between tiers or how the penalty behaves. Once the first finding is fixed, can you run a regularVisiblePlan on a simulator where the tree tier fails fast and the sweep is slow, and show snapshotQuality.backend on that snapshot, the AGENT_DEVICE_RUNNER_SNAPSHOT_XCTEST_CHANNEL_PENALIZED line in runner.log, and snapshotQuality.backend=privateAX on the next snapshot of the same screen?

Smoke Tests and Repo Guards were still running with no failure excerpt captured; Smoke covers the runner snapshot capture plan and the ios.yml workflow change, and Repo Guards covers the XCTest file and test list touched by the new UnitTests files, so a failure in either should be treated as possibly related to this diff.

Before this is ready to merge, please push the slice-deadline sweep fix so it arms the penalty and falls through to privateAX, move the target-identity reads to main, add a plan-level test for each, and then run the live simulator validation described above.

thymikee · 2026-09-23T20:12:50Z

Both re-opened findings are addressed at 5385c2a44f, test-first. The branch is rebased on origin/main (b3b58caac1), so commit 1 now carries main's snapshot route instead of the removed one.

Finding 1 — a deadline-exhausted query sweep was not treated as a tier timeout. The tier stopped starting queries at its slice deadline but the plan classified only what it collected, and sparsePayloadReason accepts anything above the sparse-node threshold. A partial flat sweep therefore became a recovered capture: no XCTest-channel penalty, private-ax never reached, and every later capture of that screen paid for the sweep again. runFlatInteractiveQueries now returns a typed SnapshotTierOutcome, querySweepSnapshotAcquisition carries it, and snapshotTierRejectionReason(outcome:kind:payload:) rejects .deadlineExhausted as a timeout — the payload survives only as the best/firstFailure fallback, and snapshotXCTestPenaltyReason arms <backend>_backend_timeout from the same typed outcome rather than from a message. Red before the fix (RunnerTests+SnapshotCapturePlanOccupancyTests.swift:343, :349, :354); green now, plus testDeadlineExhaustedTierIsRejectedWhileItsIdenticalCompletedPayloadIsAccepted and testDeadlineExhaustedTierAttemptArmsTheChannelPenaltyWithoutBeingSlow (0.019 s — the classification is proven without waiting on a clock).

Finding 2 — the coordinate tap's routing probe keyed a penalty with target identity read off main. applyMainOwnedSnapshotState and the lifecycle code write currentBundleId through DispatchQueue.main.async, so the probe could penalize a target main was still clearing or rebinding. The probe now takes a SnapshotProbePenaltyTarget: a snapshot hands over .prepared(bundleId:) — the identity it already took on main — and the tap route passes .mainOwnedTarget, letting the probe's own main-side block capture the identity main holds once its work starts (SnapshotProbePenaltyIdentity, one write, lock-guarded). The probe itself stays off main, which testExecuteDispatchedReturnsBusyBeforeBlockingSystemModalProbeDrains still requires. Red before the fix (RunnerTests+CommandExecution.swift:706, :710); green now. I also enumerated the remaining production currentBundleId reads on the dispatch path: hasCachedTargetForActivationSkip, resolveAppWithoutActivation, prepareActiveCommandContext, tapInAppBackControl and executeTypeCommand all run inside a main hop already, so the probe was the only off-main reader left.

One limit, stated plainly: the live triple you asked for (tree tier fails fast + slow sweep, then reason=queries_backend_timeout, then private-ax) cannot be driven through the CLI on a stock surface — the tree tier answers first, and the only ways to make it fail fast without arming a penalty are a real kAXErrorIllegalArgument refusal or regularSnapshotMaxNodes (50 000) presented nodes; there is no fault-injection seam in the runner. The on-simulator lane does plant exactly that state (RunnerSlowSweepQueryStub), and that is where the chain is proven: penalty reason, deferred XCTest tiers on the next plan, and private-ax answering. The live pass I could run — 107/107 iOS PR lane, and a real open/snapshot/press text="Updates"/close cycle on a simulator created and deleted for this run — covers the tap route and the capture path without regression. Details in the Validation section.

thymikee · 2026-09-23T22:36:05Z

Reviewed at 5385c2a. Both findings from the earlier pass (#2836 (comment)) are fixed: snapshot-plan target state stays on main, and the query sweep is bounded by its slice. This is ready to merge.

All 18 checks are green. The iOS runner-regressions job runs the two new tests, which exercise the changed capture-plan and tap-routing code, so its result covers this diff.

I did not run the on-simulator tests or read the CI logs, so the "red before the fix" claims rest on tracing the pre-fix code rather than a reproduced failure. The plan-level test depends on RunnerSlowSweepQueryStub yielding more than 8 nodes before the slice ends, and I did not verify that count myself. The plan-level proof starts at runSnapshotCapturePlan, not at the daemon or executeSnapshotDispatched route; the author says no CLI-reachable fault-injection seam exists for that entry point, so I'm relying on the on-simulator lane as the live evidence for this runner-internal path.

Not blocking: on the .mainOwnedTarget route, if main stays occupied past the 4 s probe slice by untracked work, captureFromMain never runs and onAbandoned arms the penalty with a nil bundleId, which isSnapshotXCTestChannelPenalized then applies to every target instead of one — worth a bundle=<unknown> log line or a note in the commit, but can be taken or left.

…ites The capture plan runs on the command queue but read currentBundleId, currentAppProcessIdentifier and the penalty warm-up exemption, and wrote runnerAccessibilityHealth and invalidated the cached target directly. Snapshot preparation now takes a SnapshotCaptureTarget on main (app, bundle id, pid, consumed warm-up exemption) and the plan, the modal probe penalty, the XCTest penalty recorder and the private AX tier read only that copy. Health writes and the fail-closed invalidation go through applyMainOwnedSnapshotState, the abandoned-work-guarded main hop that invalidateCachedTargetAfterSnapshotFailure now also uses. Fixes #2781

A non-interactive query sweep used Date.distantFuture as its own deadline, so only the up-to-20 s plan deadline bounded it while the caller abandoned the tier after its 1 s slice. XCTest queries cannot be cancelled, so the sweep kept the main thread busy and later commands got RUNNER_BUSY. The caller now derives one slice deadline (querySweepSliceDeadline) for both its wait and the sweep, for interactive and non-interactive requests alike. Neither the element queries nor the per-element reads start with less than flatInteractiveQueryBudget left before that deadline (querySweepCanStartQuery). Fixes #2783

…that runs Taking the snapshot target on main consumed the exemption during command preparation, so a snapshot answered by the blocking system-modal probe, or an abandoned preparation that ran late, spent it without running a plan. The exemption now lives in a lock-owned type that lifecycle code arms on main and runSnapshotCapturePlan consumes at its start.

…ugh the plan Swizzles XCUIElementQuery.allElementsBoundByIndex with a slow query and runs a non-interactive [.querySweep] plan with a 20 s plan deadline. The sweep must not be abandoned, and no query may start after the plan answers, so passing the plan deadline to the sweep in either loop turns it red.

…ity to main Two regressions #2781 still leaves open, both red on this head: A query sweep that stops at its own slice deadline is a tier timeout, but the plan classified its payload by node count, so a partial sweep that beat the sparse threshold armed no XCTest-channel penalty and no later capture of the same screen was deferred. The coordinate tap's system-modal routing probe armed its penalty with currentBundleId read on the command queue, while main owned that state and could still be clearing or rebinding it.

…er timeout The sweep tier stopped starting queries at its slice deadline, but the plan classified only what it collected. sparsePayloadReason accepts anything above the sparse node threshold, so a partial flat sweep became a `recovered` capture: no XCTest-channel penalty was armed, private AX never ran, and every later capture of that screen paid for the full sweep again. `runFlatInteractiveQueries` now returns a typed `SnapshotTierOutcome`, the sweep tier carries it into the attempt, and the plan rejects a `.deadlineExhausted` tier through `snapshotTierRejectionReason` — keeping its payload only as the fallback, arming the penalty through the existing tier timeout reason, and letting the next backend answer.

The coordinate tap resolved its system-modal routing on the command queue and armed an abandoned probe's penalty with `currentBundleId` read there, while `applyMainOwnedSnapshotState` and the lifecycle code write that identity through `DispatchQueue.main.async`. A tap could penalize a target main was still clearing or rebinding. The probe now takes a `SnapshotProbePenaltyTarget`: a capture hands over the identity it already took on main, and the tap route hands over nothing and lets the probe's own main-side block capture the identity main holds once its work starts. No off-main path reads target identity any more.

main (#2854, #2855) moved the runner's inline unit tests into UnitTests/ and split RunnerTests+CommandExecution.swift along command families. Rebase conflicts dropped this PR's edits from the inline blocks the rebase kept on main; relocate them into the file main now owns for each family: - RunnerTests+SnapshotTests.swift: the query-sweep slice-deadline and fail-closed-invalidation tests, and the bounded-modal-probe helper now takes a SnapshotCaptureTarget. - RunnerTests+SnapshotCapturePlanTests.swift: the tier-timeout rejection pair and the private-AX depth helper now builds a SnapshotCaptureTarget. - RunnerTests+SnapshotTimingTests.swift: the deadline-exhausted tier penalty test.

thymikee · 2026-09-24T06:33:06Z

Reviewed at 7414149. The rebase keeps the reviewed behavior from 5385c2a. The snapshot-plan target state stays on main, and the query sweep stops at its slice deadline. The rebased patch matches the reviewed one, apart from one added type annotation. No code on main is reverted. The new commit moves this PR's tests into main's split UnitTests files, and no stale callers remain.

I did not build or run the runner XCTest bundle. CI's iOS runner job for 7414149 confirms that the moved tests compile and pass.

Smoke Tests is still running and has not failed. This diff touches the iOS snapshot and tap-routing paths, so please check that job before merge.

github-actions · 2026-09-24T07:06:12Z

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-24 07:06 UTC

thymikee force-pushed the fix/2781-snapshot-plan-main-state branch from 8f69c76 to 9719773 Compare September 23, 2026 17:18

thymikee force-pushed the fix/2781-snapshot-plan-main-state branch from 9719773 to 5385c2a Compare September 23, 2026 20:12

thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 23, 2026

thymikee added 8 commits September 24, 2026 07:57

thymikee force-pushed the fix/2781-snapshot-plan-main-state branch from 5385c2a to 7414149 Compare September 24, 2026 06:07

thymikee merged commit eabe119 into main Sep 24, 2026
18 of 19 checks passed

thymikee deleted the fix/2781-snapshot-plan-main-state branch September 24, 2026 07:05

Navigation Menu

Uh oh!

fix(ios-runner): keep snapshot-plan target state on main and bound the query sweep by its slice - #2836

fix(ios-runner): keep snapshot-plan target state on main and bound the query sweep by its slice#2836
thymikee merged 8 commits into
mainfrom
fix/2781-snapshot-plan-main-state

thymikee commented Sep 23, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 23, 2026 •

edited

Loading

Uh oh!

thymikee commented Sep 23, 2026

Uh oh!

thymikee commented Sep 23, 2026

Uh oh!

thymikee commented Sep 23, 2026

Uh oh!

thymikee commented Sep 23, 2026

Uh oh!

thymikee commented Sep 24, 2026

Uh oh!

Uh oh!

github-actions Bot commented Sep 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

thymikee commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Validation

Uh oh!

github-actions Bot commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Size Report

Uh oh!

thymikee commented Sep 23, 2026

Uh oh!

thymikee commented Sep 23, 2026

Uh oh!

thymikee commented Sep 23, 2026

Uh oh!

thymikee commented Sep 23, 2026

Uh oh!

thymikee commented Sep 24, 2026

Uh oh!

Uh oh!

github-actions Bot commented Sep 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

thymikee commented Sep 23, 2026 •

edited

Loading

github-actions Bot commented Sep 23, 2026 •

edited

Loading