Skip to content

feat(interaction)!: remove target-readiness waits - #3599

Merged
thymikee merged 8 commits into
mainfrom
t3/drop-readiness-wait-3077
Oct 14, 2026
Merged

thymikee merged 8 commits into
mainfrom
t3/drop-readiness-wait-3077

Conversation

@thymikee

@thymikee thymikee commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

Summary

Removes the replay target-readiness wait, per the #3077 decision: the audited hosted evidence showed no rescue on either platform. Replayed press/click/longpress no longer poll for a missing target; post-gesture and scroll observation are unchanged. Scripts that need to wait use an explicit step:

wait 'label=Continue'
press 'label=Continue'

Breaking for Node clients that passed readinessTimeoutMs (shipped in v0.21.19). The key is now retired: the Node client (at its projection seam) and MCP/CLI input (through the common row) refuse it on every command with INVALID_ARGS and "add an explicit wait for the target" instead of dropping it silently. Also removed: the capture_sparse refusal (only the poll produced it; replay now reports selector_not_found on an empty tree, like live commands) and five observeUntil options only the wait used. 62 files, 533+/2,106−; the gross budget is waived for removals. Leftover result forwarding is tracked in #3607.

Closes #3077

Validation

Head 1f26100da, with main merged at 562e798d1.

  • pnpm check:affected --base origin/main --run: pass (685 files / 5,151 tests).
  • Retirement tests (Node press, click, longpress, fill, swipe; MCP press and fill) fail when either refusal site is removed.
  • Fresh-context review and cubic findings fixed.
  • Live evidence on pre-rebase source f22ddb765: Android full fixtures plus six replays (87 actions, first attempt, zero healing); iOS diagnostic six replays (47 actions). A later app-switcher fixture failure is outside the corpus and occurs without this change.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.21 MB 5.21 MB -7.1 kB
Package (unpacked) 5.21 MB 5.21 MB -7.1 kB
Package (download) 1.57 MB 1.57 MB -2.1 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 27.5 ms 27.3 ms -0.2 ms
CLI --help 78.8 ms 76.9 ms -1.9 ms

@cubic-dev-ai cubic-dev-ai Bot 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.

All reported issues were addressed across 59 files

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

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread website/docs/docs/client-api.md Outdated
Comment thread website/docs/docs/replay-e2e.md Outdated
Comment thread src/commands/interaction/runtime/__tests__/test-utils/index.ts Outdated
Comment thread packages/selectors/src/selector-pipeline-policy.ts Outdated
Comment thread src/commands/interaction/runtime/interaction-snapshot-capture.ts Outdated
@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://callstack.github.io/agent-device/pr-preview/pr-3599/

Built to branch gh-pages at 2026-10-14 16:50 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@thymikee

Copy link
Copy Markdown
Member Author

Commit 20fb8a6 has one problem to fix before merge. The change itself is a clean deletion, and I found no smaller design.

readinessTimeoutMs shipped as a Node client option in v0.21.19. The PR removes its field declaration but does not declare it through retiredField (command-input.ts). So a JS caller who passes { readinessTimeoutMs: 2000 } to press, click or longpress has the key dropped without any signal. They get one attempt, and loading screens will fail with intermittent selector_not_found. Only TypeScript users see the break, at compile time. The new test in client-interactions-input-filtering.test.ts pins that silent drop, and client.test.ts sets the opposite precedent for the retired screenshot maxSize. The rule: every released input key that a command removes is declared as retiredField on that command's field map. Here that means readinessTimeoutMs on clickFields, pressFields and longPressFields, with a message that points to wait <selector> before the interaction. Please replace the client test with an assertion that the call is rejected with INVALID_ARGS before transport, like the one at client.test.ts:1011.

Is there a smaller shape than this? I looked and found none. Routing the removed key through retiredField costs a few lines and replaces the silent-drop test.

Not blocking, and you can take or leave these: 'readiness' in SAFE_CAUSE_DETAIL_KEYS (session-replay-runtime-failure-response.ts) and its test at session-replay-runtime-failure-response.test.ts:55 are now dead, because nothing emits details.readiness. Also, the 1,000-line budget is waived only for refactor(move) PRs per docs/agents/pull-requests.md:33, so the body could ask a maintainer for a waiver instead of citing a rule.

I reviewed 20fb8a6. The newer head 3105851 only updates wording, comments and the press test for the six inline threads, and it does not touch the retiredField problem above. The same "resolve their target once" wording still appears in src/commands/schema/cli-help.ts:169, if you want help to match the docs.

CI is still running: 14 of 22 checks are queued or in progress at 20fb8a6, and none has failed. Integration and Coverage exercise the changed replay and interaction routes, so they matter once they report. There are no conflicts.

I did not open the hosted Android and iOS run logs. They ran on f22ddb7, before the sparse-refusal restore, the observeUntil trim and the rebase onto #3471. I accepted them because the head's live and replay routes are main's existing one-attempt route. I also did not trace whether the Node client filter for undeclared keys is the same projection that retiredField refuses through, though the maxSize precedent suggests it is. I could not tell whether the new press-selector-resolution scenario would fail on main, so it reads as a characterization test.

Before merge, please declare readinessTimeoutMs as retiredField on the three field maps and update the client test. Then Integration and Coverage need to report green.

@thymikee

Copy link
Copy Markdown
Member Author

The code has one problem to fix before merge, and the 22 checks at 3105851 are green. I found no other defect in this change.

Main refused readinessTimeoutMs as a declared input on press, click and longpress, and refused it on every other command through COMMON_INPUT_FIELDS. This head deletes both and declares nothing, so readFieldInput now drops the key. A call like client.interactions.press({selector, readinessTimeoutMs: 2000}) goes out as a one-attempt press with no error. The same happens on MCP and any other surface, and fill and other commands now drop a key they used to refuse. A Node caller on v0.21.19+ who used the released option will see intermittent selector_not_found on loading screens, with no signal that the option is gone. The new test in client-interactions-input-filtering.test.ts locks in the silent drop. The rule should be: a released input key that was removed is refused with migration guidance on every command input path. retiredField and the retired audience in command-input.ts own this, as with the maxSize retirement on screenshot and record. Could you declare readinessTimeoutMs as a retired field in the click, press and longpress maps with a message that says to add an explicit wait for the target? One unconditional refusal row in COMMON_INPUT_FIELDS would also cover every command from a single site. Then please change the new test to assert INVALID_ARGS with that message for press, click and longpress, and for at least one MCP call. This also covers what the maintainer raised on 20fb8a6.

Not blocking, and you can take or leave these: the PR removes the only producer of result.readiness, but interaction-touch-response.ts:236 still copies it, the 'readiness' entry in SAFE_CAUSE_DETAIL_KEYS in session-replay-runtime-failure-response.ts still forwards it, and the tests that assert both still build that shape.

The six cubic-dev-ai threads are fixed at 3105851, so you can resolve them: docs wording in client-api.md, dispatched assertion in press-selector-resolution.test.ts, replay-e2e.md wording, test-utils comment, promotedTarget wording and moving-target-gate.ts mention.

On evidence, I did not run the MCP path for an unknown key, so whether MCP strict schemas reject or drop readinessTimeoutMs is inferred. The live Android and iOS replay runs are from before the rebase. I did not open their logs, because the later commits do not touch device dispatch. An older client that sends flags.readinessTimeoutMs to a newer remote daemon will have it ignored, and I did not look into that version skew further.

Before merge, readinessTimeoutMs needs to be refused with migration guidance on Node and MCP, and the silent-drop test needs to become a refusal test.

thymikee and others added 4 commits October 14, 2026 12:53
The sparse-capture refusal existed only inside the readiness poll. Moving it
into the shared failure path widened it to every live selector action, so
restore the one-attempt behaviour and drop the capture_sparse reason that no
longer has a producer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ait used

budgetFrom, captureDeadline, rideOut, signal and phase have no remaining
production caller after the readiness wait's removal.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…solution

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n guidance

Removing the option let readFieldInput and the tap writers drop it silently, so a
released caller's press became a one-attempt press with no signal. Declare it
retired at the common input row and refuse it in the click, press and longpress
writers that the Node client reaches directly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@thymikee
thymikee force-pushed the t3/drop-readiness-wait-3077 branch from 3105851 to 40a5cb0 Compare October 14, 2026 11:05
@thymikee

Copy link
Copy Markdown
Member Author

Addressed the review of 3105851 at 40a5cb0b1 (rebased on main ba7730af0):

  • readinessTimeoutMs is retired, not dropped. A retired row in COMMON_INPUT_FIELDS refuses it on every command read through readFieldInput (MCP, CLI JSON). The click, press and longpress daemon writers refuse it too, because the Node client calls those writers directly. The message: readinessTimeoutMs was removed; add an explicit wait for the target before the interaction.
  • The silent-drop test is now a refusal test for Node press, click and longpress (INVALID_ARGS, no transport call). The MCP test covers press and fill on the real command route. Each test fails when its guard is removed.
  • input-audience.test.ts now scopes its operator-key invariant to operator rows and asserts that retired keys are advertised on no schema.
  • The non-blocking result.readiness and SAFE_CAUSE_DETAIL_KEYS leftovers are tracked in Remove interaction readiness result forwarding left after the readiness-wait removal #3607 so this PR stays scoped.

Gate: pnpm check:affected --base origin/main --run passed (683 files / 5,141 tests).

@thymikee

Copy link
Copy Markdown
Member Author

The PR is ready at 40a5cb0. The silent drop of readinessTimeoutMs from the earlier review (3105851) is fixed: Node and MCP now refuse it, with tests for both. All 22 checks pass, and I know of no conflicts. Nothing else blocks the merge, so it needs a maintainer to merge it.

Not blocking, and you can take or leave these: assertNoRetiredReadinessTimeout in https://github.com/callstack/agent-device/blob/40a5cb0/src/commands/common-input-fields.ts#L93 repeats the reader body of retiredField(message).read in command-input.ts, so it could call that reader or share one refuseRetiredKey. The Node path skips readFieldInput, so each retired key needs a hand-placed guard (five sites now, starting at https://github.com/callstack/agent-device/blob/40a5cb0/src/commands/interaction/interactions.ts#L125), and a Node fill or hover call with readinessTimeoutMs still drops the key silently, as on main. A follow-up could refuse any input key whose audience is retired once, in prepareDaemonCommandRequest, and delete the per-writer guards.

The six inline threads from cubic-dev-ai no longer apply, so you can resolve them: #3599 (comment) (client-api.md no longer says the target is resolved once), #3599 (comment) (the press test asserts details.dispatched === 'no'), #3599 (comment) (replay-e2e.md wording was fixed), #3599 (comment) (the test-utils comment was fixed), #3599 (comment) (the promotedTarget wording was fixed), and #3599 (comment) (the snapshot capture comment now names moving-target-gate.ts).

I did not run the tests. I judged the regression coverage from the diff and the earlier writers at 3105851. An older client (v0.21.19-21) that sends flags.readinessTimeoutMs to a newer remote daemon still gets a one-attempt press with no error, because I found no daemon-side refusal like retiredScreenshotMaxSizeFlagError for maxSize. That only affects clients talking to a remote daemon of a different version. The live Android and iOS replay runs predate the rebase, but this change is client-side input validation only and does not touch device dispatch, so I do not ask for a new live run.

@thymikee

Copy link
Copy Markdown
Member Author

Merged main in at e419d18.

  • src/commands/interaction/runtime/resolution.ts: kept this PR's removal of the readiness poll path (single attemptSelectorResolution call); dropped main's readinessScheduleFor import.
  • src/commands/interaction/runtime/selector-readiness.ts: stays deleted; main's fix(selectors): type rect-requiring targets as addressable #3608 change (SelectorResolution to ActionSelectorResolution) was ported to selector-target-resolution.ts, which now holds those types.

@thymikee

Copy link
Copy Markdown
Member Author

The earlier review of 40a5cb0 was clean, and I checked the new head e419d18 against it. All six inline comments from the earlier review are fixed at this head, and the code looks right. Required evidence is in place: the live Android and iOS runs, but those ran on the pre-rebase source f22ddb7. The merge changes no PR-side behavior (it removes one import in resolution.ts), so I am not asking for a re-run. I did not run typecheck or tests on the merged head. I judged the merge resolution from the diffs and grep only.

Smoke Tests is still queued, so there is no CI result yet. Its replay and interaction route overlaps the press, click and longpress resolution change in this PR, so it is the check that matters here. The next step before merge is for Smoke Tests to finish green. After merge, #3607 removes the leftover readiness forwarding.

One question on simplicity. The retired-key refusal is enforced twice: in the common input row (MCP, CLI and batch), and again by hand in the click, press and longpress Node-client writers, because the Node client goes from prepareDaemonCommandRequest straight to the writer. Node fill or swipe calls with readinessTimeoutMs still drop the key silently, although the PR body says every command refuses it. Could the Node projection seam (prepareDaemonCommandRequest) refuse every key whose COMMON_INPUT_AUDIENCE kind is 'retired' in one place, and could the row reuse retiredField's check? That would replace the three per-writer guards and the copied hasOwn and throw. Otherwise, should the PR body narrow the claim to press, click and longpress?

The six earlier cubic-dev-ai threads are fixed at this head, so the author can resolve them: client-api.md wording #3599 (comment), the dispatched assertion in press-selector-resolution.test.ts #3599 (comment), replay-e2e.md wording #3599 (comment), the test-utils clock comment #3599 (comment), the promotedTarget doc #3599 (comment), and the consumer list in interaction-snapshot-capture.ts #3599 (comment).

…ction seam

The Node client reaches daemon writers without readFieldInput, so per-writer
guards covered only click, press and longpress. Refuse every retired common key
in prepareDaemonCommandRequest instead, and share one refusal between
retiredField and the common row.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@thymikee

Copy link
Copy Markdown
Member Author

Took the design point from the review of e419d18, at 1f26100da:

  • Retired common keys are refused once in prepareDaemonCommandRequest, the Node client's projection seam, by iterating COMMON_INPUT_AUDIENCE for retired entries. This replaces the three per-writer guards in click, press and longpress, so Node fill, swipe and every other command now refuse readinessTimeoutMs as well.
  • retiredField and the common row share one refuseRetiredInput helper in input-audience.ts, so the copied hasOwn-and-throw is gone. interactions.ts is back to identical with main.
  • The Node test now also covers fill and swipe, and it fails if the seam call is removed. The PR body states the claim per surface.

Gate: pnpm check:affected --base origin/main --run passed (685 files / 5,151 tests).

@thymikee
thymikee merged commit ecd7b45 into main Oct 14, 2026
22 checks passed
@thymikee
thymikee deleted the t3/drop-readiness-wait-3077 branch October 14, 2026 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Readiness wait: hosted evaluation and keep/drop decision

1 participant