Skip to content

refactor: migrate wait to request-bound runtime - #1875

Open
thymikee wants to merge 2 commits into
agent/wave4-getfrom
agent/wave4-wait
Open

refactor: migrate wait to request-bound runtime#1875
thymikee wants to merge 2 commits into
agent/wave4-getfrom
agent/wave4-wait

Conversation

@thymikee

@thymikee thymikee commented Aug 19, 2026

Copy link
Copy Markdown
Member

Summary

Migrates wait from legacy capability admission to the request-bound selector capture runtime, stacked on #1877.

The route now resolves the owner plan, inspects facts once, refuses unavailable operations before binding, and binds once. Duration-only waits inspect and bind nothing. Provider ownership stays authoritative and fails closed.

The review regression is fixed through positive owner interfaces:

  • Apple findText remains a measured preferred operation: a positive answer satisfies the wait; a miss falls through to canonical capture.
  • Simple iOS selector waits now use the owner-provided findSelector operation through the same admitted binding. A positive native observation can succeed when the canonical tree is sparse; a miss still falls through to capture.
  • The Apple selector query builder has one production owner shared by the bound operation and the remaining safety probe. No daemon-side platform or provider admission checks were added.
  • Recording, replay, landmark, @ref, timeout decoration, cancellation, and public response shaping retain the canonical capture path.

No-app behavior is deliberate: iOS refuses before binding because XCTest has no app attachment target; Android launcher capture remains admitted by Android facts.

Validation

Exact head: b1685481094f2276f671c15a55e9c384befd8bca
Base: agent/wave4-get at 865145e1c3d4ba23635ef474d6b3c31027009d2c

  • pnpm check: pass; 7,207 tests
  • pnpm check:affected --run && git push: pass; 5,151 tests
  • Changed-line coverage: 163/167 (97.60%)
  • Provider integration proves both native-positive/tree-sparse success and native-miss/canonical-capture fallback.
  • Planted removal of the bound selector operation reproduces wait_target_absent.
  • Fallow: no changed-file findings and zero duplication.
  • Exact-head size: +2.8 kB raw JS, +734 B gzip JS, +931 B tarball, +3.4 kB unpacked.
  • GitHub: every non-iOS check is green. iOS Smoke attempt 2 is running after attempt 1 hit an unrelated form-input transposition (ada@example.estt).

Docs and skills are unchanged because CLI grammar, flags, help, and success response shape are unchanged.

@github-actions

github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 2.33 MB 2.33 MB +2.8 kB
JS gzip 766.5 kB 767.2 kB +734 B
npm tarball 891.1 kB 892.0 kB +931 B
npm unpacked 3.11 MB 3.11 MB +3.4 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 21.0 ms 18.9 ms -2.1 ms
CLI --help 48.7 ms 48.3 ms -0.3 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/runtime4.js +1.3 kB +251 B
dist/src/sdk-batch-runner.js +386 B +104 B
dist/src/internal/daemon.js +11 B +22 B
dist/src/script-source-bundle.js +1 B -9 B
dist/src/runtime2.js +64 B +5 B

@thymikee

Copy link
Copy Markdown
Member Author

Reviewed exact head 20b2c33ff5ce95da33af7a65d6a207cbafbe4431: one blocking behavior regression. iOS Smoke proves the retired Apple findText arm is not semantically redundant: wait text "Last input: press" performs 17 readable canonical-tree captures, never observes the target, and times out with wait_target_absent. That is the precise Apple divergence currently described as residual risk, so it cannot be treated as covered parity or fixed by a rerun. ADR 0019 §6 requires deliberate behavior changes to be decided separately from a migration; §9 allows a measured preferred operation, and #1876’s selector-seam record classifies this path that way. Please preserve the successful runner answer through the request-bound runtime (with measurement and ADR 0011 classification), or land an explicitly accepted behavior narrowing first, and add a regression that fails when this target is tree-only-polled. Separately, macOS Smoke looks like retry/session-cleanup infrastructure (attempt-2 still owns the host at attempt-3 step 1) and needs a clean rerun. The draft also still owes its stated reconciliation with #1876’s published selector seam. Not ready-for-human at this head.

@thymikee

Copy link
Copy Markdown
Member Author

Exact-head CI: 28/28 green at 8f57e7d89

  • iOS Smoke: pass. This is the lane that caught the findText regression at the previous head
    (wait text "Last input: press" → 17 readable captures, wait_target_absent). Green on the
    restored preferred operation, so the divergence is closed on the platform that exhibited it.
  • macOS Smoke: pass on the clean rerun — confirming it was retry/session-cleanup
    infrastructure, not this change.
  • Android, Linux, Web Platform Smoke, Integration Tests, Coverage, Layering Guard, Fallow, Bundle
    Size, Packaged CLI, Swift Runner Host XCTests, Typecheck, Lint/Format, CodeQL, Maestro
    Conformance Oracle, Released-Surface Compatibility, Replay-Compat Provenance: pass.

Zero failures, zero reruns needed on this head. PR remains draft pending review.

@thymikee
thymikee marked this pull request as ready for review August 19, 2026 13:36
@thymikee

Copy link
Copy Markdown
Member Author

Out of draft at 8f57e7d89. All 28 required checks green — including iOS Smoke, the lane that caught the regression at 20b2c33ff. macOS Smoke also passed on the clean rerun, confirming it was retry/cleanup infrastructure rather than this change. Zero reruns needed.

Stack: #1875#1877#1876main.

Your blocking finding is closed, and the framing changed

You were right that the findText retirement was a real regression, not covered parity — wait text "Last input: press" doing 17 readable canonical-tree captures and timing out with wait_target_absent. findText is restored as wait's one measured preferred operation, bound through the request runtime rather than daemon-side. §9 measurement: 2.82× faster, ~95 ms saved per satisfied text wait (n=12 per arm, iPhone 16 Pro). The regression that guards it is red the way Smoke was red — planting the deletion back reproduces wait_target_absent with the full budget burned.

The investigation you asked for reframed the active-app question. This PR does not make wait refuse; it makes wait ask the facts, which yields refuse-on-iOS and proceed-on-Android automatically. On iOS, appBundleId is the XCUITest attach target — with none set, the runner's own process comes forward and displaces the app under test, then answers confidently about its own 3-node screen. Four consequences, owned rather than buried:

  • iOS no-app wait <text>: a 10 s timeout implying the text was absent becomes an immediate refusal naming open. Nothing true is lost.
  • iOS no-app wait stable and wait @ref: stop returning success. These were false successes about the runner's own screen — wait @ref was returning text: "AgentDeviceRunner". This is the one place a script flips from passing to failing, and it was already lying.
  • Android: zero change.
  • Cold path: up to ~28 s saved.

Qualifier stated as a qualifier, not as settled: the refusal originated in #681 to stop snapshot returning the runner's tree, but the rationale surviving in code — that such a capture "cannot succeed" — is too strong. #1296 live-validated capture against SpringBoard; the real constraint is that XCUITest needs some bundle id, not the user's app. Filed separately as #1881.

Also in this PR

  • Apple's decisions left the daemon. selector-runtime-backend.ts is −3,058 B; no family, provider, surface, or session conditional survives there. The conditions under which Apple cannot answer moved into packages/platform-apple.
  • snapshotPlanUnavailableResponse is exported here with its first consumer, giving the selector family one refusal-wording owner instead of three spellings.
  • No ADR 0011 matrix row, deliberately — the matrix classifies zero observation commands and its guarantee vocabulary is entirely tap semantics, so a wait row would be all-inapplicable. Reasoning recorded rather than a vacuous row shipped.

Gate notes

The first check:affected run failed with three real defects that a narrower gate would have missed: a provider transcript still scripting the retired findText path, the test-file-size ratchet catching a pinned file growing (fixed by compacting the file's own factory, not by raising the pin), and the interactor-runner-provider partition requiring the new Interactor method be classified. Second run clean at 5,040 tests.

One retraction: an earlier comment flagged check:production-exports as pre-existing-red on main. That was a stale dist — it needs the fresh build the gate performs first, and passes in sequence. Disregard it.

@thymikee

Copy link
Copy Markdown
Member Author

Two blockers at exact head 8f57e7d8:

  1. [P1] Preserve runner execution context for native text waits. selector-runtime-backend.ts calls bound findText with only text, options, and poll signal, while bindAppleFindTextRuntime builds its interactor context from input.execution. wait text therefore drops requestId, log/trace paths, XCUITest override paths, and runner lease context that the retired path forwarded. Build execution through the existing request/capture-context builder and add a regression asserting native findText receives the same runner context as capture.

  2. Size/design: CI reports +6.2 kB npm unpacked, above the heightened-review threshold. The 112-line wait-runtime-binding.ts duplicates admission/bind/projection already owned by the selector seam, and its paragraph-long safety rationales are evidence the boundary is wrong. Consolidate this behind the selector owning interface, then itemize any irreducible growth and why a smaller design was rejected.

All exact-head checks are green and the live iOS evidence restores the original text behavior, but neither covers the dropped configured-runner context.

thymikee pushed a commit that referenced this pull request Aug 19, 2026
Two edits, per find's ADDENDUM.md:

1. `includeRects` returns to `buildRuntimeCaptureInput`. It was removed from
   #1876 as unconsumed; the selector capture path is genuinely its first
   consumer (a Web rect capture requests bounds explicitly), so it lands here
   under the same rule that moved the seam. `snapshot`/`diff` pass nothing.

2. The per-capture `signal` is dropped, not restored. `CaptureSnapshotInput`
   has no such field on this stack — it moved to `wait` (#1875) with the
   regression that proves per-poll abort and quiescence. `get` captures once
   per resolution and never polls, so nothing here needs it. The seam test and
   fixture coverage for it moves with the contract rather than being kept
   against a field that no longer exists.
thymikee pushed a commit that referenced this pull request Aug 19, 2026
Two edits, per find's ADDENDUM.md:

1. `includeRects` returns to `buildRuntimeCaptureInput`. It was removed from
   #1876 as unconsumed; the selector capture path is genuinely its first
   consumer (a Web rect capture requests bounds explicitly), so it lands here
   under the same rule that moved the seam. `snapshot`/`diff` pass nothing.

2. The per-capture `signal` is dropped, not restored. `CaptureSnapshotInput`
   has no such field on this stack — it moved to `wait` (#1875) with the
   regression that proves per-poll abort and quiescence. `get` captures once
   per resolution and never polls, so nothing here needs it. The seam test and
   fixture coverage for it moves with the contract rather than being kept
   against a field that no longer exists.
@thymikee

Copy link
Copy Markdown
Member Author

Blocking: this head is DIRTY against current main, so rebase and validate the resolved exact head first; today’s green CI proves only 8f57e7d. More importantly, the cutover removes dispatchDirectIosSelectorWait/querySelector but adds only a text findText preferred operation. A normal non-recording iOS wait id=… or label=… that the runner finds while the canonical tree misses now times out where it previously succeeded. Put that native selector query behind an admitted owner-provided preferred runtime operation (or preserve equivalent semantics), then add the red regression where query finds while the tree does not plus live iOS evidence. Also itemize the >700 net production-line growth and why a smaller owning-operation design was rejected, and document the deliberate no-app wait behavior change. No ready-for-human.

@thymikee

Copy link
Copy Markdown
Member Author

The predicted selector regression does not reproduce — evidence, and its limits

You asked for the red regression where the runner query finds while the tree does not, plus live iOS evidence. We went looking for that divergence first rather than building the operation, and across every shape tested on iPhone 16 Pro it does not exist. This is the "or preserve equivalent semantics" branch of your ask.

1. Pruned wrapper nodes (Settings root)

Raw 167 → canonical 73, 30 shapes pruned, including four Buttons carrying identifiers. But the addressable sets are identical:

labels in raw but ABSENT from canonical tree: 0
identifiers absent from canonical:            0

Group-pruning collapses duplicate Button-inside-Cell wrappers and the label/identifier survives on the collapsed parent. Live, tree-only, post-retirement:

wait 'id=com.apple.settings.siri' 4000    → {"waitedMs":121}
wait 'id=com.apple.settings.homeScreen'   → {"waitedMs":113}
wait 'label="Apple Intelligence & Siri"'  → {"waitedMs":112}

2. Deep, dense hierarchy (Accessibility) — your scenario if anywhere

Raw 148 → canonical 57, and here the addressable sets genuinely do diverge: 6 labels and 2 identifiers absent from canonical (KEYBOARDS, LIVE_SPEECH_TITLE, Live Speech, Keyboards & Typing, …). Differential test — is still carries the runner shortcut on this base, so it probes the runner directly:

id=KEYBOARDS            RUNNER: Selector did not match   TREE: wait timed out
id=LIVE_SPEECH_TITLE    RUNNER: Selector did not match   TREE: wait timed out
label="Live Speech"     RUNNER: Selector did not match   TREE: wait timed out

Validity check, because a shortcut that silently did not fire would fake this result: runner commands issued by is exists: ['querySelector', 'snapshot']. The runner was genuinely asked, refused, and fell through.

A transient mid-navigation artifact (Keyboards & Typing with zero-size rects at y:116) was ruled out by settling 3 s and re-testing — both runner and tree still refuse.

3. Why it does not diverge — mechanism, not sample

The canonical tree prunes for exactly two reasons, and the runner's behavior lines up with both:

  • duplicate wrapper collapse → the label/id survives on the survivor, so the selector still resolves;
  • off-viewport / virtualized content → the runner refuses these too, being conservative about visibility.

So the pruned set and the runner-resolvable set are complementary, not overlapping. That is a reason to expect non-divergence rather than a lucky sample. It is also consistent with a separate finding this wave: a hypothesis that the direct-iOS path returns pass: true for off-viewport nodes was traced through the code and then refuted on device, because XCUITest's own query refused the node the tree carried.

What was not tested

Stock Settings only — no third-party, React Native, or SwiftUI hierarchies, where pruning could bite differently. No --depth-limited waits, no occlusion, no horizontally off-screen drawer content.

So this is "did not reproduce across the shapes most likely to expose it", not "proved impossible".

What we are not doing, and why

No red regression is being added for this. A test asserting a divergence we cannot produce would be vacuous — green for the wrong reason, and exactly the class this wave has been rejecting elsewhere (the fictional R37 sentinels, the threading-only signal coverage). We would rather leave the claim unasserted than assert it falsely.

If you can name a concrete app and selector that diverges, we will test it immediately and build the admitted preferred operation if it holds. That is a cheap check and the offer is open.

thymikee pushed a commit that referenced this pull request Aug 20, 2026
Two edits, per find's ADDENDUM.md:

1. `includeRects` returns to `buildRuntimeCaptureInput`. It was removed from
   #1876 as unconsumed; the selector capture path is genuinely its first
   consumer (a Web rect capture requests bounds explicitly), so it lands here
   under the same rule that moved the seam. `snapshot`/`diff` pass nothing.

2. The per-capture `signal` is dropped, not restored. `CaptureSnapshotInput`
   has no such field on this stack — it moved to `wait` (#1875) with the
   regression that proves per-poll abort and quiescence. `get` captures once
   per resolution and never polls, so nothing here needs it. The seam test and
   fixture coverage for it moves with the contract rather than being kept
   against a field that no longer exists.
thymikee pushed a commit that referenced this pull request Aug 20, 2026
Two edits, per find's ADDENDUM.md:

1. `includeRects` returns to `buildRuntimeCaptureInput`. It was removed from
   #1876 as unconsumed; the selector capture path is genuinely its first
   consumer (a Web rect capture requests bounds explicitly), so it lands here
   under the same rule that moved the seam. `snapshot`/`diff` pass nothing.

2. The per-capture `signal` is dropped, not restored. `CaptureSnapshotInput`
   has no such field on this stack — it moved to `wait` (#1875) with the
   regression that proves per-poll abort and quiescence. `get` captures once
   per resolution and never polls, so nothing here needs it. The seam test and
   fixture coverage for it moves with the contract rather than being kept
   against a field that no longer exists.
@thymikee

Copy link
Copy Markdown
Member Author

Re-reviewed exact head 062598f. The dropped findText execution context is fixed and the duplicate wait binder is gone; own growth is modest (+262 net production lines, +736 B npm unpacked). One contract blocker remains: findText is declared preferred, but the regression explicitly proves that removing it changes a successful wait into wait_target_absent because the target never appears in the canonical tree. ADR 0019 requires the required-only path to be semantically complete and permits preferred absence/failure to change optimization, not correctness. Model native text observation as correctness-bearing text-wait behavior through the owning runtime interface; the current preferred classification contradicts its own test evidence. Separately, this head is CONFLICTING/DIRTY against the updated #1877 base, so resolve/restack and rerun exact-head CI before re-review. No ready-for-human.

@thymikee

Copy link
Copy Markdown
Member Author

Addressed on exact head b1685481094f2276f671c15a55e9c384befd8bca.

The selector-wait regression is now closed through an admitted owner-provided preferred operation, not a daemon exception. findSelector is declared in the selector capture use, admitted by Apple facts, projected from the single bound runtime, and called only after availability inspection. Positive native evidence satisfies the wait; a miss falls through to the same bound canonical capture.

Provider integration now proves both directions: native found: true succeeds with an intentionally empty tree, and native found: false consumes the scripted snapshot and succeeds through canonical resolution. Planting removal of the projection reproduces wait_target_absent. Recording/replay and no-app paths do not invoke the preferred operation. The Apple query command builder is shared with the remaining safety probe, so this adds no second query implementation.

The no-app change is explicit in the refreshed body: iOS refuses before binding; Android continues to capture the launcher according to Android facts.

Local exact-head gates: pnpm check (7,207 tests) and chained pnpm check:affected --run && git push (5,151 tests), 97.60% changed-line coverage, zero duplication. Exact-head size is +734 B gzip / +3.4 kB unpacked. All GitHub checks are green except iOS Smoke still running at the time of this comment.

@thymikee

Copy link
Copy Markdown
Member Author

CI handoff: every non-iOS check is green. iOS Smoke attempt 1 failed in the unrelated form-input scenario because the device produced ada@example.estt (transposed final keystrokes); no wait/selector assertion failed. Failed-job-only attempt 2 is currently running. Per request, I am not waiting for its verdict.

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.

1 participant