Skip to content

fix(apple-runner): a penalized snapshot plan falls back to the bounded XCTest probe - #3051

Merged
thymikee merged 5 commits into
callstack:mainfrom
okwasniewski:oskar/penalized-snapshot-falls-back-to-xctest
Sep 29, 2026
Merged

thymikee merged 5 commits into
callstack:mainfrom
okwasniewski:oskar/penalized-snapshot-falls-back-to-xctest

Conversation

@okwasniewski

@okwasniewski okwasniewski commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Once the XCTest channel is penalized for a bundle, the regular snapshot plan on a simulator is private AX alone. When private AX cannot match the app, nothing else runs, and the capture comes back as the sparse fallback with a zero root. Presentation then refuses it: regular iOS snapshot presentation requires a valid viewport. Private AX fails to match the app with Could not match active AX application for XCTest application, which is what happens while the out-of-process "Save Password?" sheet is over it. That repeats on every capture for the whole 120 s penalty.

This was the top flake in the e2e mobile benchmark. Login Form failed on its first pass in about 20 of 45 CI runs: every snapshot after submit failed, the agent could not see the sheet's Not Now, and it gave up. Runner log:

SNAPSHOT_XCTEST_CHANNEL_PENALIZED bundle=dev.e2e.benchmark reason=queries_backend_timeout
SNAPSHOT_XCTEST_CHANNEL_DEFERRED bundle=dev.e2e.benchmark
PRIVATE_AX_SNAPSHOT_FAILED=Could not match active AX application for XCTest application

The penalized plan now keeps the tree, on the bounded probe's 1 s slice that physical devices already use, behind the independent backend: [privateAX, recursiveTree]. The query sweep stays off, since its grind is what penalizes the channel. A private-AX capture that succeeds returns first, so nothing changes on screens where it works. A capture the bounded tree recovered reports budget instead of the seeded deferred.

Touched: 1 runner source file, 1 unit test file.

Release note

While the XCTest channel is penalized, a capture the bounded tree recovered carries reasonCode: "budget", not deferred, and shows the recovered warning on each such capture, as physical devices already do. So does a deferred plan that ends sparse after the tree ran.

Validation

Tested commit 82a354807 (live run, check:affected, xctest selection reaches all 344 methods); 4b3971da7 after it only lets the tree-recovery test skip when the bounded tree misses its slice.

  • Live, iOS 26.5 simulator, loaded host: the mobile benchmark's model-driven Login Form reached the Save Password sheet on a penalized bundle. The runner log shows CHANNEL_PENALIZED, then CHANNEL_DEFERRED, then PRIVATE_AX_SNAPSHOT_FAILED (Could not match active AX application), then SNAPSHOT_RECOVERED backend=tree. The snapshot came back as recovered / tree / budget with the sheet in the tree, and there was no viewport refusal. Details in the thread.
  • Before, without this change: 1 of 3 loaded Login Form runs failed with the persistent viewport refusal. After, on c6c67d5ed: 18 of 18 passed unthrottled.
  • New simulator-lane tests use a unit-test-only private AX override: recovered/tree/budget, and sparse/budget when the tree tier blocks. Both fail with the stamping reverted.
  • RunnerTests+SnapshotCapturePlanTests pins the penalized plan as [.privateAX, .recursiveTree] and the verdict-reason rule.

Review in cubic

Copilot AI lite review requested due to automatic review settings September 28, 2026 21:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 2 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

@okwasniewski

Copy link
Copy Markdown
Contributor Author

Both red jobs look unrelated to this change, which only touches the snapshot capture plan. I can't rerun them (no admin rights on the repo):

  • Coverage failed in src/daemon-client/__tests__/daemon-client-transport.test.ts:213, a remote-daemon sendWithStaleInstance test. This PR touches no TypeScript.
  • iOS Smoke Tests failed in testSynthesizedReplacementPacesAnAppOwnedFieldAtItsAcknowledgeWindow: 11 edits reached the app in 327 ms (closest pair 4 ms), below the 400 ms average. That is the keystroke pacing inside a type command, which the test's own comment says XCTest does not space evenly. The snapshot plan is not on that path.

Could a maintainer rerun the failed jobs?

…, and names the budget

The query sweep's grind is what penalizes the channel, so it stays off a
deferred plan; the occupancy test's no-sweep contract holds again. A
capture the bounded tree recovered reports 'budget', not 'deferred'.
Copilot AI review requested due to automatic review settings September 29, 2026 00:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@thymikee

Copy link
Copy Markdown
Member

The fix looks right in code, but I can't call it ready yet, because the only live run is on an earlier commit and not on 3719be2. Thanks for the focused change.

This is a device-facing runner change, and the live results (1/3 fail before, 4/4 pass after) are for fc1a335. Head 3719be2 changed the plan (querySweep is gone from the fallback) and the verdict reason (budget instead of deferred) at https://github.com/callstack/agent-device/blob/3719be2/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift#L272. The daemon latch (src/daemon/snapshot-quality-latch.ts:56) and the warning suppression (quality-warnings.ts:58) both key on that reason. So the output on the fixed route now differs from what was validated. Please run the loaded iOS simulator Login Form scenario on 3719be2, or any capture while an out-of-process sheet covers a penalized bundle. The runner log should show SNAPSHOT_XCTEST_CHANNEL_PENALIZED, then SNAPSHOT_XCTEST_CHANNEL_DEFERRED, then PRIVATE_AX_SNAPSHOT_FAILED (Could not match active AX application), then SNAPSHOT_RECOVERED backend=tree. Please also show snapshot --json with snapshotQuality state recovered, backend tree, reasonCode budget, with no "requires a valid viewport" refusal and no SNAPSHOT_TIER_* querySweep start in that window.

Not blocking, and you can take or leave these: a verdict should say budget whenever an XCTest tier ran on the bounded slice, but recoveredVerdictReason enforces that only on the accept path (line 453), so if private AX fails and the bounded tree times out or is rejected as sparse, the terminal path still stamps the seeded deferred reason (you could seed budget from the plan itself whenever it contains an XCTest tier); the new test at RunnerTests+SnapshotCapturePlanTests.swift:433 only calls the pure helper, so removing the verdictReason wiring at line 428 would leave every test green (if the occupancy harness can stub backends, a plan-runner test with private AX failing and the tree accepted should assert recovered/tree/budget); and tree-recovered captures now show the full recovered warning on every capture during the penalty, because the latch only one-shots deferred, which matches physical-device budget behavior but is a user-visible change with no CHANGELOG entry.

CI is green, with 14 checks and none failing. The earlier Coverage and iOS smoke failures are on routes this Swift-only diff does not touch. I read the code at 3719be2 but did not run the Swift tests or the simulator. Before merge, we need the live simulator run on 3719be2 that reaches the Save Password sheet route and shows the tree recovery stamped recovered/tree/budget.

…the budget on every path

The sparse terminal path kept the seeded 'deferred' reason after the
bounded tree ran. Both paths now stamp 'budget' once an XCTest tier ran
behind private AX. A unit-test seam lets plan-runner tests make private
AX read nothing, as it does behind an out-of-process sheet, and pins
recovered/tree/budget and sparse/budget.
Copilot AI review requested due to automatic review settings September 29, 2026 09:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@okwasniewski

Copy link
Copy Markdown
Contributor Author

Thanks @thymikee. Live run on the current head 82a354807, plus your three non-blocking points.

Live run. The mobile benchmark's agentic Login Form was driven by the model (--no-cache) on an iOS 26.5 simulator, against 82a354807 built from source. The host was loaded (every core busy) and the simulator's processes were throttled to background QoS, to recreate CI's slow host. The Save Password sheet came up, the channel was penalized, and the runner log shows the sequence you asked for:

11:02:56.179 SNAPSHOT_XCTEST_CHANNEL_PENALIZED bundle=dev.e2e.benchmark reason=tree_backend_timeout
11:02:56.180 SNAPSHOT_TIER_SKIPPED_XCTEST_OCCUPIED tier=queries
11:03:28.930 SNAPSHOT_XCTEST_CHANNEL_DEFERRED bundle=dev.e2e.benchmark
11:03:31.507 PRIVATE_AX_SNAPSHOT_FAILED=Could not match active AX application for XCTest application
11:03:31.647 SNAPSHOT_RECOVERED backend=tree reason=XCTest-backed snapshot tiers are running with a short recovery probe after recent slow accessibility work on this screen
11:03:31.822 SNAPSHOT_XCTEST_CHANNEL_DEFERRED bundle=dev.e2e.benchmark
11:03:34.643 PRIVATE_AX_SNAPSHOT_FAILED=Could not match active AX application for XCTest application
11:03:34.808 SNAPSHOT_RECOVERED backend=tree reason=XCTest-backed snapshot tiers are running with a short recovery probe ...

The snapshot response the client received for that capture (its snapshotQuality, plus a count of nodes and whether the sheet's controls are in the tree):

{"snapshotQuality":{"state":"recovered","backend":"tree","reason":"XCTest-backed snapshot tiers are running with a short recovery probe after recent slow accessibility work on this screen","reasonCode":"budget","timing":{"acquisitionMs":138.6,"presentationMs":0.34}},"nodes":19,"appBundleId":"dev.e2e.benchmark","savePasswordSheet":true}

There was no requires a valid viewport refusal, and no query-sweep tier started after the deferral; the only queries line is the skip above, in the plan that armed the penalty. The scenario passed. Across every run I did on c6c67d5ed and 82a354807 there was not one viewport refusal.

For full transparency: most other runs in that throttled setup failed. They failed on text entry, app install and runner timeouts (secret-fill timed out, installApp exceeded 30000ms, a dropped password character), never on a snapshot. By then other sessions had pushed this host's load average to about 50 on 15 cores, and a fresh reboot didn't help, so I don't read those runs as saying anything about this diff. Without the throttling, on c6c67d5ed, the same scenario passed 18 of 18 (8 replayed, 10 model-driven).

The terminal path now stamps budget (c6c67d5ed). Once an XCTest tier ran behind a deferred plan, planVerdictReason returns the bounded-probe reason on both the accept path and the sparse terminal path. The SNAPSHOT_RECOVERED log line now prints that same reason (82a354807).

Plan-runner tests. A unit-test-only privateAXAcquisitionOverrideForTesting makes private AX read nothing, following the pattern of systemModalProbeOverrideForTesting. Two new tests in RunnerTests+SnapshotCapturePlanOccupancyTests.swift:

  • testPenalizedPlanRecoversThroughTheBoundedTreeWhenPrivateAXReadsNothing expects recovered, tree and budget.
  • testPenalizedPlanThatEndsSparseAfterTheBoundedTreeReportsTheBudget blocks the tree tier and expects sparse and budget.

Both pass 3 of 3 on an iOS 26.5 simulator with -D AGENT_DEVICE_RUNNER_UNIT_TESTS. With the stamping reverted to firstFailure in the runner, both fail. check:affected passes, and check:xctest-selection reaches all 344 methods.

Release note. The repo has no CHANGELOG file, so the user-visible change is in the PR body. While the penalty lasts, captures the bounded tree recovered carry budget and show the recovered warning each time, as physical devices already do, not the one-shot deferred warning.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 4 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic

…covery test

On a host too loaded for the 1 s slice the plan ends sparse; the verdict
must still say budget, and the recovery assertions skip with the reason.
Copilot AI review requested due to automatic review settings September 29, 2026 09:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@thymikee

Copy link
Copy Markdown
Member

The code at 4b3971d looks right, and the new occupancy tests pin the plan well. It still needs a live run on this head before it is ready. Since 3719be2 the fallback plan changed again: the penalized plan now checks window occupancy before it picks the bounded XCTest probe (RunnerTests+SnapshotCapturePlan.swift). The live results in the thread are for fc1a335, so they do not cover this route.

Could you run the loaded iOS simulator Login Form scenario on 4b3971d, or any capture while an out-of-process sheet covers a penalized bundle? The runner log should show SNAPSHOT_XCTEST_CHANNEL_PENALIZED, then SNAPSHOT_XCTEST_CHANNEL_DEFERRED, then PRIVATE_AX_SNAPSHOT_FAILED, then SNAPSHOT_RECOVERED backend=tree, and snapshot --json should report snapshotQuality recovered, backend tree, reasonCode budget.

CI is green and there are no conflicts.

@thymikee

Copy link
Copy Markdown
Member

Correction to my previous comment on 4b3971d: please ignore the request for a new live run. The live run already covers this head.

This looks good. The earlier concern is fixed: the live run now covers the production code at 4b3971d (the changes since 82a3548 touch tests only), and all 14 checks are green with 0 failing. Not blocking: on a slow host the bounded tree can miss its slice and the plan ends sparse, so the recovery test in RunnerTests+SnapshotCapturePlanOccupancyTests.swift (https://github.com/callstack/agent-device/blob/4b3971d/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+SnapshotCapturePlanOccupancyTests.swift#L398) skips before its recovered and tree assertions and only the budget reason is checked there. You can take or leave a longer slice for the tree in that test so the recovered branch always runs. I did not run the Swift unit tests or the simulator. The 3/3 pass and red-on-revert claims are yours, and I only checked them by reading the pre-change code. The live run was on a throttled host. You report that the text-entry and install failures in other runs are unrelated, and I can only confirm that none of them was a snapshot refusal on your word. No conflicts.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 29, 2026
@thymikee
thymikee merged commit 1df5a7a into callstack:main Sep 29, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants