Skip to content

fix(selectors): retire the stored tree after a side effect or a native read - #2869

Merged
thymikee merged 1 commit into
mainfrom
fix/stale-selector-cache-after-navigation
Sep 24, 2026
Merged

thymikee merged 1 commit into
mainfrom
fix/stale-selector-cache-after-navigation

Conversation

@thymikee

@thymikee thymikee commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

Summary

Selector reads (get, is, find, wait) reuse the session's stored tree for up to 750 ms after it was captured. Two events outdated that tree without replacing it:

  • A wait text poll missed natively, captured and stored the old screen, then got a native findText hit.
  • A mutation dispatched without capturing (a coordinate press, for example).

In both cases the next read reused the pre-change tree in about 1 ms. It then returned the old value or Selector did not match. This is the local smoke:automation-input failure at get text id="automation-event-name".

Fix: every side-effect seam (expireRefFrame) and every native text read marks the session's current tree outdated. The session cache refuses an outdated tree. The mark is keyed by the tree, so it survives session-record copies. The per-session runtimeRevision resets on those copies, and session-selector-dispatch.ts makes one right after its seam. 7 files, including CHANGELOG and selector-capture.md.

Closes #2866

Validation

Tested SHA eab7f34d. pnpm check:affected --run passes.

  • Tests through the production handlers:
    • wait-runtime.test.ts: wait text, then get. Fails if the findText mark is removed.
    • interaction-touch-press.test.ts: is, then coordinate press, then get. Fails if the seam mark is removed.
  • Live, iOS 26.2 sim, CI fixture app:
    • wait text repro: origin/main failed 8/12; this SHA failed 0/12 (the earlier findText-only head failed 0/27).
    • Batch get→press→get: seam mark removed, 4/6 stale (Last input: none in 1 ms); fix, 0/7.
  • assertAutomationInput ×10: every startup read passed. 2 runs failed later in the separate long-press/scroll bucket.

@github-actions

github-actions Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.80 MB 4.80 MB +160 B
Package (unpacked) 4.80 MB 4.80 MB +160 B
Package (download) 1.44 MB 1.44 MB +52 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 17.1 ms 17.1 ms -0.1 ms
CLI --help 48.5 ms 48.4 ms -0.1 ms

@thymikee
thymikee force-pushed the fix/stale-selector-cache-after-navigation branch from b17286d to eab7f34 Compare September 24, 2026 08:24
@thymikee thymikee changed the title fix(selectors): retire the stored tree when a native read observes the device fix(selectors): retire the stored tree after a side effect or a native read Sep 24, 2026
@thymikee

Copy link
Copy Markdown
Member Author

Addressed the blocking finding in eab7f34d, which replaces the earlier head.

  • Invariant: the session cache tier serves a tree only if no side-effect seam and no native read happened after the tree was stored. expireRefFrame marks the session's current tree outdated. findText calls the same markSessionSnapshotOutdated. reusableSessionSnapshot has one check, isOutdatedObservation. The findText-specific WeakSet in the capture runtime is deleted.
  • Why a tree mark and not a revision stamp: runtimeRevision is a WeakMap keyed by the session object. A spread copy of the session resets it to 0, and session-selector-dispatch.ts:230 makes such a copy right after its seam at :86. A tree stamped at revision 0 would then match again. A mark keyed by the tree survives the copy. A stamp would also need every snapshot writer to stamp, and an unstamped tree would silently lose reuse.
  • Native reads do not advance runtimeRevision: a read-only wait text does not change the device, so internal-observation.ts:147 still publishes refs after it.
  • Press test: a selector press captures interactiveOnly, which is a different presentation key from get. So press→get with a selector press was already safe. The test uses the reachable shape instead: is (stores the full tree), then a coordinate press (no capture), then get. It is in interaction-touch-press.test.ts, not interaction-get.test.ts. Touching that file makes fallow's audit flag an unbaselined neighbour test.
  • Mutations: removing the seam mark fails the press test. Removing the findText mark fails the wait-text test.
  • Live, own sim:
    • Batch get→press→get with the seam mark removed: 4/6 stale (Last input: none, 1 ms). With the fix: 0/7.
    • wait text repro on this SHA: 0/12.

@thymikee

Copy link
Copy Markdown
Member Author

Reviewed at eab7f34. The code looks correct: a side effect that does not capture, or a native read that answers without capturing, now retires the stored tree, so the next get or is captures again.

Not blocking, take or leave:

  • The CHANGELOG names get, is, find and wait, but wait, find and is absent always capture fresh through needsFreshSnapshot. Only get and non-absent is reuse the stored tree (CHANGELOG.md#L5).
  • docs/agents/selector-capture.md could limit "a native read ... retires the tree" to a native read that answers without capturing, such as wait text's findText. readTextAtPoint and other native probes do not mark the tree (selector-capture.md#L13).
  • The stored-tree invariant asserted in interaction-touch-press.test.ts (#L396) belongs in a selector-capture-runtime test.

The live numbers (0/12 at this head against 8/12 on main, and 0/7 against 4/6 for the batch run) are author-reported. Smoke Tests, Repo Guards and Coverage were still running. Smoke runs the wait text then get path this PR changes, so a failure there would need a look.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 24, 2026
…e read

Selector reads reuse the session's stored tree for 750 ms after its
capture. Two events outdated that tree without replacing it: a wait text
satisfied by the owner's native text reading after an earlier miss
capture, and a mutation that captured nothing. The next get or is then
reused the pre-change tree and reported Selector did not match.

Every side-effect seam (expireRefFrame) and every native text reading now
marks the session's current tree outdated, and the session cache tier
refuses an outdated tree. The mark is keyed by the tree, so it survives
the session record copies that reset the per-session runtime revision.

Closes #2866
@thymikee
thymikee force-pushed the fix/stale-selector-cache-after-navigation branch from eab7f34 to a9b3878 Compare September 24, 2026 09:19
@thymikee

Copy link
Copy Markdown
Member Author

Rebased onto main at a9b3878bab. The only conflict was CHANGELOG (the #2863 entry landed above ours), and both entries are kept. pnpm check:affected --run passed on this head.

@thymikee

Copy link
Copy Markdown
Member Author

Reviewed at a9b3878. The rebase keeps the change from the earlier clean review at eab7f34: the stored tree is retired after a side effect or a native read. The only conflict was the CHANGELOG, and both entries are kept.

The two Smoke Tests jobs were still queued at review time.

@thymikee
thymikee merged commit fd24123 into main Sep 24, 2026
19 checks passed
@thymikee
thymikee deleted the fix/stale-selector-cache-after-navigation branch September 24, 2026 09:35
@github-actions

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-24 09:36 UTC

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.

A selector read right after wait text can answer from the tree captured before the screen changed

1 participant