fix(mobile): cancel presses when the response wait expires by shamilovtim · Pull Request #1685 · e2e-inc/e2e

shamilovtim · 2026-11-11T01:32:44Z

Summary

Cancel pending SDK interactions.press calls when the run is interrupted or their response wait expires. Keep the existing 250 ms acknowledgement allowance and nonretryable timeout result.
Route every press through InputBudget.press and pass the whole operation to the budget factory, so callers cannot omit the signal forward. No new test API.
Limit cancellation to taps, double taps, focus, check and uncheck actions, and the press calls used to focus a field or dismiss a keyboard tip. Other input requests retain their existing lifetimes. Canceling a promise does not prove an accepted native hold, gesture or text sequence finished; extending cancellation to those requests needs a separate drain policy. Recording finalization is unchanged.

flowchart LR
  A[Operation signal] --> B[Input budget]
  B --> C[SDK press]
  D[Response wait expires] --> E[Abort SDK request]
  C -- answers --> F[Return original result]

Issue

Fixes #1684. Related: #1682 and callstack/agent-device#4388.

Verified

Ran it locally: yes

Built CLI, e2e run --video on --trace on, on a constructed UIKit fixture. The same test and SDK build reproduce late input on main and prevent it here.

main `d8b694f`	this branch `c282cf9`
Tap fails at 502 ms, then lands after the three-second overlay finishes. The unchanged zero-input assertion fails.	Zero input after the overlay finishes; the immediately following attempt receives exactly its own one tap. All three cases pass.

Main failure video · Fixed timeout video · Next attempt video

Exact commands, tests, native events and all five videos.

The package still pins agent-device 0.21.36, used by the SDK boundary tests. The native overlay comparison deliberately uses the separately identified readiness candidate 965e77b and matching native assets on both builds. It is not a claim that the published SDK already has readiness polling.

Accepted native iOS input was verified separately on the same SDK/native build: main with recording off, this branch with recording off, and this branch with recording on. After the native runner accepted a double tap, caller cancellation was followed immediately by the next attempt's app relaunch and one tap. All three cases preserved the device ordering; each fresh app received exactly its own tap. With recording off, this branch admitted the next attempt locally before native completion, while the SDK fenced subsequent device work. Three videos, native event logs and exact hashes.

The iOS SDK runner owns that bounded drain (#3383, #3385, already in 0.21.36). An original command deadline, unavailable status or notAccepted can leave native work unconfirmed; rejecting the client promise is not a universal termination guarantee. Arbitrary custom transports that bypass the daemon are outside this native proof.

At d45a6e15, the branch added test coverage to the unchanged production source from e15c0585. All 524 mobile tests and the full repository check pass in a separately installed checkout with pinned agent-device 0.21.36. The coverage proof records all eight isolated press-route bypass mutations failing the new assertions, plus the fresh device-fixture budget signal delivering one late input and failing the zero-input oracle; restoring the source passes.

The native cancellation videos above retain their original tested commits and SDK identifiers. Those dedicated probes were not repeated for the structural revision or this test-only follow-up.

Android ordering is supported by the pinned 0.21.36 source, separately from those iOS recordings: existing requests hold both session and device keys; abort rejects queued work but does not release an admitted task. Android tap and the outer request wrapper await the backend promise. The next relaunch therefore remains queued while that promise is pending. This is backend-promise ordering, not a universal native-completion guarantee. Version-pinned code trace.

Head 66957b62 adds documentation/test nits only. The exact reviewed patch was validated in a separately installed checkout: 247 focused tests, all 524 mobile tests, full pnpm check and mobile/workspace type checks pass. Committed bytes are identical to the tested patch; no Android native run or new native video is claimed. Source, checks and attribution.

Current-head CI is terminal: 25 successful checks and 3 skipped, including successful iOS/Android deterministic suites and all runnable gates. Automated review reports zero issues across five files. Maintainer re-review remains pending. Earlier native recordings and coverage controls retain their original source attribution.

cubic-dev-ai

All reported issues were addressed across 7 files

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

View guided diff | Re-trigger cubic}

shamilovtim · 2026-11-11T01:47:37Z

[GPT-6] responding on behalf of shamilovtim

All checks are green at 31ceef35325ffbc14ef02cc1fd0893baf4e0b4c0, with no unresolved review threads. Applying the Ready for Human Review label was denied because this account lacks AddLabelsToLabelable permission. A maintainer can add the label.

okwasniewski

Thanks. The core fix is right: checked against agent-device 0.21.36, aborting the per-request signal closes the connection and the daemon marks the request canceled, the deadline path still returns ACTION_MAY_HAVE_COMMITTED, and the 250 ms acknowledgement allowance survives (the ack test kills the mutation that drops MIN_ANSWER_MS). A few changes before this is reviewable:

Must fix

packages/mobile/src/support.ts:80-85: the .catch remap is dead. deadline is only aborted inside onTimeout, and the timeout rejection is queued before the SDK's request_canceled rejection, so the timeout always wins. Instrumented: the deadline case receives ACTION_MAY_HAVE_COMMITTED; a caller abort receives COMMAND_FAILED request_canceled, which the branch excludes. Removing the branch or loosening its conditions fails no test. Drop it and the isAgentDeviceError import (it also couples us to the SDK error-detail shape).
Put the operation signal in InputBudget, not at every call site. inputBudget(label, timeoutMs) is already built per operation (engine.ts:37-58, device.ts:319). Pass the operation signal to it and have send always dispatch with AbortSignal.any([opSignal, deadline.signal]). That removes the eight (signal) => ..., 0, operation.signal call sites, the 0 placeholder for holdMs, the extra signal parameter on pressKey (surface.ts:1539), and the pre-abort check at support.ts:72 (duplicates raceAbort's precheck; removing it fails no test). It also fixes the untested wiring: dropping the operation signal from performAt, pressKey, dismiss-Continue, focus or check fails no test today; only tap is tested for caller abort.
Docs and changeset. docs/mobile.mdx:772-774 and .changeset/mobile-cancel-pending-press.md say cancellation "stops native readiness polling". The pinned 0.21.36 has no such polling (that is the unreleased candidate in callstack/agent-device#4388). "Press requests", "adapter" and "possibly-committed timeout behavior" are internal terms. One user-facing line is enough, e.g. "A tap, double tap, focus or check still pending when its timeout expires, or when the run is interrupted, is cancelled on the device instead of landing late."

Should fix

Coverage is uneven: fill, type, keyboard enter, longPress, swipe, root scroll, drag and back still send uncancelled, and inside pressKey the focus tap is cancelled but the keystroke after it is not (surface.ts:1556-1559). With (2) this is one argument per call site. Either extend it or say in the body why holds and gestures are left out (a long press lifted early on abort is the only reason I can see).
~30 engine.test.ts assertions gained signal: expect.any(AbortSignal) because the fake client records signal in args. Recording it separately in tests/helpers/fake-client.ts keeps those assertions about behavior.
The ack-allowance test (input-cancellation.test.ts:62+) exercises InputBudget directly, so it belongs in support.test.ts. Its type case never uses the signal.
The PR body says "Independent review cleared the change"; please drop that line.

Mutations (16 on src hunks): 9 killed. Survivors: the catch remap (3 variants), the pre-abort check, and the operation signal on performAt/pressKey/dismiss-Continue/focus/check. (2) makes the last group impossible by construction.

For maintainers

Should cancellation cover every input or only zero-hold ones?
track() used to make the next attempt wait until an abandoned press finished; now the aborted promise rejects at once and ordering relies on the daemon's session lock. On iOS, after a hold timeout or unconfirmed status, the next request can start before the canceled work ends. Acceptable?
With published 0.21.36 the user-visible effect is small (a press queued behind a stuck command no longer lands late); the main payoff comes with agent-device#4391.

This was an automated review.

shamilovtim · 2026-11-11T05:11:27Z

[GPT-6] responding on behalf of shamilovtim

Addressed at c282cf9099c907adf581872d0cf57e74b70719ac:

Removed the dead catch remap, SDK error-shape import, and redundant pre-abort check.
InputBudget now requires the operation signal at construction and composes it with each response deadline. Removed the per-send signals, zero hold placeholders, and pressKey signal parameter.
Corrected the docs and changeset for the pinned published SDK. The body explicitly identifies the unreleased SDK used by the native reproduction and no longer claims independent review cleared the change.
Kept cancellation scoped to SDK press calls. Fill, typing, keyboard commands, holds, swipes, scrolls, drags and back retain their previous behavior. The field-focus press inside key entry is covered; the following key request is not. Aborting an SDK promise does not prove accepted native work drained. Session/device locks serialize daemon work but are not evidence that abandoned native holds ended, so this patch does not widen that policy.
Recorded fake-client signals separately and restored the existing assertions to their behavior payloads.
Moved the acknowledgement cases to support.test.ts; both the press and typed-error case now receive the SDK request signal. Added a later-input case for the same operation's cancellation lifetime.

All 523 mobile unit tests, mobile typecheck/build/lint, docs and package checks pass. Fresh native main-versus-head verification uses the same current SDK 965e77b and native assets: main fails the zero-input oracle after a 502 ms tap timeout, while this revision preserves zero input after the three-second fade and permits exactly one input from the next attempt. Videos and native event logs: https://github.com/shamilovtim/e2e/tree/295f841a902472f56dc5f90741fbac80d9363236/reproductions/implicit-native-press-readiness/mobile-review-c282cf9.

cubic-dev-ai

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

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

View guided diff | Re-trigger cubic}

cubic-dev-ai

0 issues found across 1 file (changes from recent commits).

_{Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.

View guided diff | Re-trigger cubic}

shamilovtim · 2026-11-11T05:51:49Z

[GPT-6] responding on behalf of shamilovtim

Head 9dc33e80fc2545e2af56519fc780590259472b77 is green across CI, including the iOS and Android deterministic suites. All review threads are handled; the accepted-native-work concern has the upstream contract and three real-native verification cases.

@okwasniewski, the requested changes and follow-up evidence are ready for re-review. The formal re-review request was denied because this account lacks GitHub's RequestReviewsByLogin permission, so the existing CHANGES_REQUESTED status remains until you re-review or it is dismissed by a maintainer.

cubic-dev-ai

All reported issues were addressed across 1 file (changes from recent commits).

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

View guided diff | Re-trigger cubic}

cubic-dev-ai

0 issues found across 2 files (changes from recent commits).

_{View guided diff | Re-trigger cubic}

cubic-dev-ai

0 issues found across 1 file (changes from recent commits).

_{View guided diff | Re-trigger cubic}

okwasniewski

[claude-opus-5-5] automated review on behalf of @okwasniewski

Thanks, re-reviewed at a4f0be5. Done since the last round:

the dead remap and the pre-abort check are gone
the signal lives in InputBudget
the docs no longer claim readiness polling
the fake client records signal separately
the ack test moved to support.test.ts

The scope note in the body (holds, gestures and text need a drain policy) works for me.

One structural thing is left. Ask (2) was meant to make it impossible to drop cancellation, but each SDK press still forwards the (signal) => argument by hand, so a forgotten forward still compiles. On this head, I removed the forward at each press site one at a time and no test failed: check, focus, locator doubleTap, the pressKey focus tap (surface.ts:1555), dismiss-Continue (surface.ts:1699) and pointer tap/doubleTap (actions.ts:71,73). Dropping operation.signal from the performAt or keyboard press budgets (engine.ts:41,45) fails no test either.

Add press(client, options) to InputBudget (send((signal) => client.interactions.press({ ...options, signal }))) and route all eight press sites through it. Then cancellation can't be forgotten, and the "press requests" sentence in the generic send JSDoc (support.ts:58-59) moves to the place where it holds.
Make inputBudget(label, operation) take { timeoutMs, signal } instead of two args. That removes the 9 repeated operation.timeoutMs, operation.signal pairs in engine.ts and device.ts:319, and the two engine-level gaps above with them.

Nits:

requestSignal at surface.ts:1555 no longer avoids a clash with anything, so name it signal like the other sites.
docs/reference/mobile.mdx says "SDK requests", which is an internal term. "the next attempt waits for them to finish" is enough.

CI: the only failure is the Timeout 2000ms assertion in the web integration test. That is #1689 / #1690, not this PR.

shamilovtim · 2026-11-11T15:57:18Z

[GPT-6] responding on behalf of shamilovtim

Addressed the 10:14 UTC re-review in e15c05852c35de4067f691d0776629fb207c4d0c:

InputBudget.press(client, options) owns the SDK signal forward. All eight press paths use it; the only direct SDK press call is inside that helper. Its JSDoc carries the press-specific cancellation and acknowledgement text.
inputBudget(label, operation) takes the timeout and signal together. Engine pointer and keyboard entry points pass the whole operation. Pointer dispatch owns its budget once, preserving existing nonpress hold limits and cancellation policy.
The field-focus callback and its requestSignal variable disappear with the shared helper. The reference now says the next attempt waits for other commands to finish.
Existing tests call the helper and updated factory. No new test cases were added.

Verified in a separately installed checkout with the pinned agent-device 0.21.36. All 523 mobile tests and the full pnpm check pass.

The existing caller-abort test catches a missing central forward. Command:

pnpm --filter @e2e-dev/mobile exec vitest run tests/unit/input-cancellation.test.ts -t 'caller abort'

Source	Result
Candidate	1 passed
Remove only `signal` from `client.interactions.press({ ...options, signal })`	Fails at `input-cancellation.test.ts:50`: expected delivered `0`, received `1`
Restore the exact candidate bytes	1 passed

The earlier native videos retain their original tested commit and SDK identifiers. This structural revision has SDK-boundary verification; the native probes were not repeated for it. The accepted-native-work qualification and bounded SDK drain policy remain as documented.

cubic-dev-ai

0 issues found across 8 files (changes from recent commits).

_{View guided diff | Re-trigger cubic}

okwasniewski · 2026-11-11T16:14:39Z

[claude-opus-5-5] automated review on behalf of @okwasniewski

Re-reviewed at e15c058. Both asks are done:

InputBudget.press(client, options) (support.ts:56) owns the signal forward, and all 8 press paths go through it.
inputBudget(label, operation) takes the operation, and performAt hands the budget to pointerInteraction with the same hold limits as before.

Defect hunt: clean. The SDK's request_canceled rejection is still handled through the race, the withTimeout timer is cleared, a deadline still maps to ACTION_MAY_HAVE_COMMITTED and a caller abort to CANCELLED, and the docs and changeset match the press paths. Mobile typecheck, lint, dead-code and all 523 mobile unit tests pass. CI is green.

Mutation results: dropping the signal in press, skipping deadline.abort(), dropping either input to AbortSignal.any, dropping MIN_ANSWER_MS, dropping track, and bypassing the helper at the locator tap all fail tests.

Optional, not blocking:

fake-client.ts:42 records signal per call, but no test reads it. A raw client.interactions.press call at any of 7 other sites (focus, doubleTap, check, pressKey focus, dismiss Continue, pointer tap/doubleTap) fails nothing. One engine.test.ts assertion that every recorded press has a signal would cover them.
device.ts:319: replacing context.signal with a fresh signal fails no test.
dismissKeyboard(signal, input) (surface.ts:1624) gets the same signal twice.

Dismissing my earlier change request and labeling this Ready for Human Review. Open maintainer questions, not yours to resolve: daemon lock ordering is iOS-only (agent-device #3385), and the scope is presses only rather than all inputs.

Asks addressed in e15c058

shamilovtim · 2026-11-11T17:56:26Z

[GPT-6] responding on behalf of shamilovtim

Addressed all three optional points from the latest re-review in d45a6e1551629f0f2dd165b739d6970a059febeb:

The recorded call.signal is now checked in the existing all-actions, pointer, and keyboard-tip scenarios. All eight press paths are covered. Bypassing the shared helper at each site, one at a time, fails the new assertion.
A new real-SDK device-fixture case holds the Continue press, aborts the fixture context, releases the transport, and requires zero delivered input. Replacing only the budget's context.signal with a fresh signal fails with one delivered input; restoring it passes.
Kept dismissKeyboard(signal, input). The explicit signal aborts the caller's waits around keyboard commands, snapshots, viewport reads and fallback swipes; the budget forwards cancellation to press requests. Removing the explicit argument would require a broader change to that plumbing. The existing nonpress request lifetimes remain intact.

All 524 mobile tests and the full pnpm check pass in the independently installed checkout. This follow-up changes tests only. The native cancellation evidence retains its original tested commit and SDK identifiers.

Exact source change, eight route mutations and fixture-signal red/green proof.

cubic-dev-ai

0 issues found across 2 files (changes from recent commits).

_{View guided diff | Re-trigger cubic}

okwasniewski · 2026-11-11T22:16:56Z

[claude-opus-5-5] automated review on behalf of @okwasniewski

Re-reviewed at d45a6e1. It only changes tests and covers the optional points:

Bypassing the shared press helper at each of the 8 sites, one at a time, now fails at least one test (7 of them used to survive).
Swapping either signal at device.ts:319 for a fresh one fails the new device-fixture case.
Keeping the explicit dismissKeyboard signal makes sense, because the outer signal also covers waits that aren't presses.

All 524 mobile unit tests and the mobile typecheck pass, the branch merges cleanly with main, and CI is green. Labeling Ready for Human Review again.

kdy1

[claude-opus-5-5] review on behalf of kdy1

The press cancellation looks right. One thing blocks it: the evidence that a cancelled press cannot overlap the next attempt covers iOS only, and the PR states that guarantee for every platform. Please add Android evidence. The other two comments are nits.

What I checked at d45a6e15:

Build, all 524 mobile unit tests, and the mobile typecheck pass.
With packages/mobile/src reverted to the merge base d8b694f, 8 of the new or changed tests fail. The 3 cases in input-cancellation.test.ts fail on delivered input, not on a missing API.
A trial merge with current origin/main (it includes #1686, which also changes mobile) has no conflicts. 537 mobile tests and the typecheck pass on it.
support.ts:57 is the only direct call to client.interactions.press.
In agent-device 0.21.36, the Android tap backend gets no signal (tap:(t,n)=>d(e,t,n) in android2.js). So the !signal branch in the tap helper does not move Android presses off the automation helper path.

Not checked: a real simulator or emulator run.

cubic-dev-ai

0 issues found across 5 files (changes from recent commits).

_{Heads up: you’ve reached your flex budget. Increase your flex budget or wait for usage to reset.

View guided diff | Re-trigger cubic}

[claude-opus-5-5] on behalf of kdy1: Addressed at 66957b6. I checked the Android lock trace against agent-device v0.21.36 (ebbaff9a) and its published dist: keyed-lock.ts rejects only work that has not yet entered the lock, and the Android tap and withRequestDeadline await their task. The docs and test nits are fixed. 524 mobile tests and the typecheck pass locally.

shamilovtim · 2026-11-13T07:06:05Z

[Codex] responding on behalf of @shamilovtim

Updated to 66957b62 following the Android review. The earlier coverage re-review still describes the unchanged production source; this follow-up changes documentation and tests only.

The Android response traces the exact published SDK's session/device locks and awaited tap promise. It satisfies the requested code-proof alternative without labeling the earlier UIKit runs as Android evidence. Backend-promise settlement remains distinct from a universal native-completion guarantee.
Addressed all three nits: precise step-stops-waiting wording, nonempty press lists at all three sites, and both support tests inside the existing group. All four threads are answered and resolved.

The exact reviewed diff passed 247 focused tests, all 524 mobile tests, full repository checks and type checks in a separately installed checkout. Committed bytes are identical to that tested patch. CI at this head passed iOS/Android deterministic suites, every runnable gate and the Node matrix; three jobs were skipped. The automated review recorded zero issues across five files. Maintainer re-review remains pending.

Reviewed source, Android trace and validation provenance.

…1612) * fix(mobile): cancel pending presses when the response wait expires Ports e2e-inc/e2e#1685 (68407d2). Co-Authored-By: Claude Sonnet 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01Ls2e1yCauKtrT9iKsfCKe9 * fix(mobile): abort a cancelled press at the client and cancel the request on every abandon Co-Authored-By: Claude Sonnet 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01Ls2e1yCauKtrT9iKsfCKe9 * fix(mobile): guard the press abort callback and order usings Co-Authored-By: Claude Sonnet 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01Ls2e1yCauKtrT9iKsfCKe9 --------- Co-authored-by: Claude <[email protected]>

shamilovtim added 2 commits November 10, 2026 19:11

fix(mobile): cancel pending press requests at the input deadline

e31a26e

fix(mobile): preserve response grace when cancelling presses

31ceef3

shamilovtim requested a review from okwasniewski as a code owner November 11, 2026 01:32

github-actions Bot added the size:M Up to 99 changed lines, not counting tests, fixtures, or the lockfile label Nov 11, 2026