fix(wait): poll through a retriable runner refusal instead of surrendering the budget - #2493
Conversation
…ering the budget The iOS Smoke lane started failing on main at the merge of #2486: the new smoke:webview-remote-content scenario ended with `wait text "Jump to form" 20000` failing after 288 ms with RUNNER_BUSY. Three defects stacked up. The scenario reused acceptDeepLinkConfirmationIfPresent, whose readiness landmark was hard-coded to the Automation lab's text. Off that route it can never match, so the helper always fell through to its `alert get` probe — and an XCTest alert query against a live WKWebView screen exceeds the runner's 30 s main-thread execution watchdog (measured 10.1 s to fail locally, 10.6 s in CI), abandoning main-thread work and leaving the runner refusing every following command as RUNNER_BUSY. The landmark is now a parameter and each caller passes its own route's, so the probe runs only when the destination genuinely did not arrive. The depth-frontier scenario carried the same mismatch and is fixed with it. A `wait` is a budgeted retry loop, but it abandoned its whole budget on the first retriable refusal. A poll whose failure the producer itself marked retriable is now ridden out like an unreadable capture: the wait keeps polling to its deadline, records the poll as `retriable` in its timeout evidence, and surfaces the refusal only when no readable capture ever completed. RUNNER_WEDGED is not retriable and still ends the wait at once. That classification was also missing on the path the failure actually took. A runner error recovered from the lifecycle journal after a lost transport response was built with a bare toAppErrorCode, so RUNNER_BUSY reached callers as a RUNNER_BUSY wire code with no `retriable` flag, while the live-response path published it as COMMAND_FAILED plus details.runnerErrorCode and retriable: true. Both paths now read the runner's code through one classifier in runner-contract. Live-validated on a booted iPhone 17 Pro simulator against the fixture app: the destination landmark resolves in 389 ms with no alert probe, the page wait succeeds in 81 ms, and the snapshot still carries Link "Jump to form", the "Email address" field label, and the remote-content-boundary XCTest fallback warning. Driving the old sequence first reproduces the wedge, after which the fixed wait polls its full 20 s in `retriable` polls instead of failing instantly.
|
Size Report
Startup median (7 runs, lower is better):
|
…recovery test The two new cases landed in runner-command-retry.test.ts, which was already over the 1,000-line test-file tripwire, so the size ratchet refused its growth. They assert runnerStatusFailureError's reading of the lifecycle journal, so their home is runner-command-recovery.test.ts, which mirrors that module and drives recovery through the real stack against a scripted fake runner.
|
A wait that spends its entire budget on retriable refusals loses the new timeout evidence at 76e5101. waitTimeoutError calls rethrowIfNeverReadable, which throws the last runner error without attaching captures, waitedMs or polls. The common all-RUNNER_BUSY case therefore cannot show where its budget went, contrary to the documented behavior. Preserve the runner code and retry details while adding the bounded wait evidence, and assert that an exhausted all-retriable wait reports its retriable polls. The shared classifier and cancellation handling otherwise look sound; iOS smoke is still running. |
…mote-content-wait-fix * origin/main: fix(android): retire completed recording evidence after pid reuse (#2487) # Conflicts: # CHANGELOG.md
|
The merge from main at aeebb3d leaves the reviewed wait code unchanged, so the missing poll-evidence finding remains open. The latest iOS smoke still fails at the WebView page wait. Its error is now correctly classified as COMMAND_FAILED with runnerErrorCode RUNNER_BUSY and retriable: true, but it still carries no wait timeline. This is the scenario the PR aims to repair, so the failure cannot be dismissed as unrelated: preserve the exhaustion evidence and diagnose why this run still cannot observe the page. |
…ation alert The 2500 ms destination budget was tuned to the Automation lab on a warm simulator. On CI the WebView lab rendered correctly but was not in the bridge tree that fast, so the helper fell through to its `alert get` probe — and that XCTest query against a live WKWebView exceeds the runner's execution watchdog, leaving every later command refused as RUNNER_BUSY. Measured on a freshly created simulator: with the confirmation alert up the probe is correct and costs 1.6 s, because the alert blocks the route and there is no web view to query; with no alert the landmark resolves in 0.1-1.7 s. The budget only has to outlast an honest mount, and overshooting it costs nothing when a confirmation really is up, since that route never renders until it is accepted. Cold-simulator run of the whole scenario: landmark 400 ms, page wait 613 ms, snapshot keeps the page link, the field label, and the XCTest fallback warning.
…fusals Review finding on #2493: a wait that spent its whole budget being refused threw the last refusal raw, so the common all-RUNNER_BUSY case carried no captures, waitedMs or polls and could not show where its budget went — contradicting the evidence this PR documents. The mirror gap existed on the other exhaustion shape: when the deadline cancelled the final poll, the wait reported a generic stall and dropped the runner code and retry details instead. Both shapes now raise one error that keeps the producer's code, message, hint and retry details and carries the wait's own evidence, with reason wait_capture_stalled and the original as its cause. A content verdict is still preserved untouched, since it already describes the capture it came from, and whether it outranks the stall verdict stays the caller's policy (wait absent). Live-verified against a genuinely wedged simulator runner: COMMAND_FAILED, retriable true, runnerErrorCode RUNNER_BUSY, reason wait_capture_stalled, captures 6, readableCaptures 0, waitedMs 8041, polls retriable,retriable,retriable,retriable,retriable,deadline.
|
Fixed in 056d014 — the finding was right, and taking it holistically turned up a mirror gap next to it. A wait can exhaust its budget two ways, and both dropped half the story:
Both now raise one error that keeps the producer's code, message, hint and retry details and carries the wait's evidence, with Live-verified against a genuinely wedged simulator runner ( Two unit tests assert each exhaustion shape, plus one that a content verdict is still returned untouched. On the smoke failure you were right not to let me wave it off. I diagnosed it rather than retrying: the WebView lab landmark had a 2.5s budget inherited from the Automation lab, the lab rendered fine but was not in the tree that fast (the failure screenshot shows it fully rendered), so the helper fell through to its |
|
No remaining findings at 056d014. All-retriable wait exhaustion now preserves the original refusal and includes poll/capture counts and elapsed time, including when the last capture reaches the deadline. The regression tests and recorded simulator run cover the prior blocker. Ready for human review; current-head CI is still running. |
thymikee
added
the
ready-for-human
1 participant
Why
The GitHub iOS workflow's Smoke Tests job has failed on
mainsince the merge of #2486, and on any PR rebased onto it. The failing step is the newsmoke:webview-remote-contentscenario:The wait was given a 20 000 ms budget and failed after 288 ms.
What actually happened
Reading the run's request logs and step history (main run 34592935964) gives the full chain:
wait for deep-link destination before inspecting system UIinspect delayed deep-link system alert(alert get)main thread execution timed outwait for the WebView page to expose its linkRUNNER_BUSYThree separate defects stack up.
The scenario asked the runner a question it should never have asked. It reuses
acceptDeepLinkConfirmationIfPresent, whose readiness landmark was hard-coded to the Automation lab's text. Off that route the landmark can never match, so the helper always fell through to itsalert getprobe. An XCTest alert query against a liveWKWebViewscreen exceeds the runner's 30 s main-thread execution watchdog, which abandons the main-thread work and leaves the runner refusing every later command asRUNNER_BUSY. I reproduced this exactly on a local simulator: the samealert gettakes 10.1 s and fails withrunner_main_thread_execution_timeout. Thesmoke:regular-visible-depth-frontierscenario carried the same landmark mismatch and paid the same doomed probe.A budgeted
waitsurrendered its budget to a transient condition.RUNNER_BUSYmeans an earlier command's abandoned work is still draining; the runner's own hint says to wait a few seconds and retry. A polling wait is that retry loop, and it quit on the first refusal.The retriability classification was missing on the path the failure took. A runner error recovered from the lifecycle journal after a lost transport response was built with a bare
toAppErrorCode, soRUNNER_BUSYreached callers as aRUNNER_BUSYwire code with noretriableflag. The live-response path publishes the identical condition asCOMMAND_FAILEDwithdetails.runnerErrorCodeandretriable: true. The repo's own policy test (retriability cannot be claimed for a code outside the enumeration) saysRUNNER_BUSYmust not be a wire code at all.What changed
acceptDeepLinkConfirmationIfPresenttakes the route's own landmark. Each caller passes thewaitpredicate for the screen it deep-linked into, so the alert probe runs only when the destination genuinely did not arrive. The WebView lab uses its nativeclose-webview-labchrome, which renders with the route and before the page.waitrides out a retriable poll failure. A poll whose failure its producer marked retriable is treated like an unreadable capture: the wait keeps polling to its deadline, records the poll asretriablein its timeout evidence, and surfaces the refusal only when no readable capture ever completed.RUNNER_WEDGEDis not retriable and still ends the wait at once.classifyRunnerReportedErrorinrunner-contract.tsnow backs both the live-response path and the journal-recovery path, so the same runner condition cannot surface under two codes with only one of them retriable.Live validation
On a booted iPhone 17 Pro simulator with the Release fixture app, driving the scenario's exact CLI steps against this branch's build:
alert getprobewait text "Jump to form" 20000The snapshot still carries
Link "Jump to form", theEmail addressfield label, and theSimulator AX snapshot unavailable (remote-content-boundary); used XCTest for this app generation.warning, so #2484's behavior is unchanged.Driving the old sequence first reproduces the wedge, and the fixed wait then polls its full 20 s in
retriablepolls instead of failing instantly, which is the ride-out working on a real refusal.Checks
pnpm lint,pnpm check:fallow,tsc --noEmit, and the touched Vitest suites pass.packages/platform-apple/src/runner/__tests__/runner-client.test.tstimes out on one unrelated spawn test under parallel load and passes when run alone; that is the known contention flake.Note on a separate local failure
Running the whole smoke suite locally fails earlier, in
smoke:automation-input, atget text id="automation-event-name". I confirmed this reproduces on pristineorigin/mainin the same environment, so it is not from this change. Thegetresolves against the session's cached snapshot in 5 ms without capturing, and locally that snapshot predates the event render. CI passes that scenario. Worth a separate issue.Closes the iOS Smoke regression from #2486.