Repository navigation
fix(settle): observe a settled screen through the capture the request admitted - #3553
Conversation
Size Report
Startup median (7 runs, lower is better):
|
There was a problem hiding this comment.
All reported issues were addressed across 22 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
… admitted --settle on back/scroll and on a coordinate-targeted press/fill polled the screen through the legacy interactor while every command the caller ran next (snapshot, diff, a selector press --settle, wait stable) read it through the request-bound runtime. On an iOS simulator those are two projections of one screen: the settled diff, the refs issued from it, and the settled: true verdict described a tree the next command could never resolve against, and a screen nobody had changed reported five additions and six removals. Both routes now admit the same capture-only plan the selector family polls under and the loop polls that one binding. An owner whose facts refuse the plan keeps observing through the interactor its own commands answer with, disclosed in the request's diagnostics.
e39e068 to
8fe9c58
Compare
There was a problem hiding this comment.
1 issue found across 8 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/daemon/generic-settle.ts">
<violation number="1" location="src/daemon/generic-settle.ts:127">
P3: This overstates the guarantee: admission or binding failure can leave the settle loop with its prior observation instead of reading through that producer. Qualify the guarantee to cases where admission and binding succeed.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
|
||
| /** | ||
| * Resolves the observation's capture before the runtime is constructed, so the loop reads the screen | ||
| * through the same producer the caller's next command reads it through (#3510). `back` and |
There was a problem hiding this comment.
P3: This overstates the guarantee: admission or binding failure can leave the settle loop with its prior observation instead of reading through that producer. Qualify the guarantee to cases where admission and binding succeed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/daemon/generic-settle.ts, line 127:
<comment>This overstates the guarantee: admission or binding failure can leave the settle loop with its prior observation instead of reading through that producer. Qualify the guarantee to cases where admission and binding succeed.</comment>
<file context>
@@ -124,15 +124,11 @@ async function observeSettled(
- * admit-and-bind for one operation, served from the gateway's binding cache rather than a second
- * device acquisition, because the observation runtime is built here and holds no reference to the
- * leaf execution's own binding.
+ * through the same producer the caller's next command reads it through (#3510). `back` and
+ * `scroll <direction>` are the cells that need it: their action use is the movement alone.
+ * `scroll top|bottom` and `scroll --until` already require `captureSnapshot` for their between-pass
</file context>
| * through the same producer the caller's next command reads it through (#3510). `back` and | |
| * through the request-bound producer when admission and binding succeed (#3510). `back` and |
|
The PR is ready at 8fe9c58, and I found nothing that needs to change before merge. All 19 checks pass at this head, and there are no conflicts. Nothing else stands between this PR and merge. Not blocking: the iOS no-effect probe at https://github.com/callstack/agent-device/blob/8fe9c58/src/daemon/interaction/internal/interaction-touch-runtime.ts#L141 gets On the open thread, the Cubic P3 about generic-settle.ts:125 still applies (#3553 (comment)). It is comment-only, and the fallback it describes is the intended, disclosed degrade. On evidence, the live A/B ran at e39e068. That commit is not in my local history, which was rewritten, so I could not confirm that this head differs from it only in tests and comments. The run covered |
|
Summary
--settleon the generic route (back,scroll) and on a coordinatepress/fillpolled the screen through the legacy interactor (XCTest runner tree), whilesnapshot,diff,wait stableand a selector--settleread it through the request-bound runtime (simulator AX bridge) — two projections of one tree. Sosettled: true, its diff, and the refs issued from it described a tree the caller's next command never resolves against:back --settlereturning to an unchanged screen reported 5 additions/6 removals, and the nextdiff snapshot -ireported that churn as real.Such a request now admits the same
capture-onlyselector-capture plan once, and the runtime resolves oneboundCaptureat construction, so a request observes through one producer; no runtime-use declaration is widened. Settling stays best-effort: an owner whose facts refuse the plan (a provider device, whose only snapshot path is the interactor its own commands answer with) keeps its previous observation and disclosessettle_observation_capture_unavailablewith its typed reason; a throwing facts probe or bind degrades the same way. An optional observation builds no command-facing refusal response, so it spends none of that response's Apple-branch host probes, and a tap without--settlepays no admission. The iOS no-effect corroboration probe polled the legacy capture too — against a keyed baseline, a tap that landed read as a recorded failure. It now answers through the request's own capture AND still forwards the baseline's backend pin: which implementation answers and which backend it presents are options of the one capture ask, not alternatives.Disclosed, not changed: an Apple session with no tracked app selects the without-active-app plan, which the Apple family refuses by declaration (its own
snapshotrefuses there too). 22 files, 258 net production lines. Closes #3510.Validation
Tested at
e39e068c3(base8255c735c):check:affected --run,typecheck,lint,format,check:layering,check:fallow --base origin/mainclean, with the device numbers below. The review round on top (8fe9c58ad) changes tests and doc comments only — no production statement — so this evidence stays attributable;typecheck,lint,format,check:layeringandfallowwere rerun on that head, and CI is the authority for the rest.iPhone 18 Pro, iOS 27.0, Settings restarted at the root before each side; 4 rounds of
press 'label=General'→back --settle→diff snapshot -ipluspress 201 130 --settle, one script at both SHAs, capture identity from per-request phase NDJSON.8255c735ce39e068c3back --settlediff, unchanged screen+5/-6(4/4)+0/-0(4/4)diff snapshot -i+6/-5(4/4)+0/-0(4/4)snapshot_capture {"backend":"xctest"}734–775 ms; 0 bridge acquiresios.snapshot-source.acquire {"producer":"simulator-ax-bridge"}210–302 mspress 201 130 --settlediffphantom+6/-5diff+0/-0back --settlewallA no-
--settletap binds only['tapPoint'], one admit-and-bind per settle request, both corroboration cells and the settled producer are asserted here; selector--settle,wait stable,snapshotand theprivate-axsparse-verdict escalation ride the unchangedcaptureSnapshotForSessionpath (snapshot-diff-runtime.test.ts), which only gains aboundCapturefield. Unresolved risks: the provider-refusal branch has no hardware coverage; thescroll top|bottom/scroll --untilcells named in the ADR amendment are covered by the shared binding cache's own tests, not by a settle test, since these cases do not bind the action leaf; the degrade is disclosed in request diagnostics atwarn(theandroid_ref_snapshot_refresh_failedprecedent), so it is visible under--verbose/--debugbut not on anok: truepayload; and a settle loop that pinsprivate-axmid-observation still crosses the runner/bridge boundary inside one observation — that pin belongs to the stable-capture owner, not this route.