Repository navigation
fix(daemon): settle the first capture after any iOS open that did not read the app - #3376
Conversation
|
There was a problem hiding this comment.
All reported issues were addressed across 13 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
Size Report
Startup median (7 runs, lower is better):
|
|
The widened trigger is not proven yet on the route that fails, so this needs one live run before merge. At 17d16c2, every iOS Simulator open that reports probe-failed, app-unidentified or not-eligible now makes the next capture run the quiet-window loop. It also pauses the direct selector tap path (deferred-interaction-outcome.ts#L123). The PR body says this was not checked live and defers proof to CI via #3374, which is still open. Nothing yet shows that the stale General tap (y=319 vs settled 406) is gone. Nothing shows what the first click after a probe-failed open now costs either. #3367 measured about 2.5 s for two settle reads, and 54 of 61 CI opens are probe-failed. Could you attach one iOS Settings replay smoke run on this head with #3374's per-step request logs? It should show three things. First, the open's response has Not blocking, and fine to take or leave: some local Simulator opens have no verdict, because The cubic-dev-ai P2 thread on the docs wording still applies: website/docs/docs/commands.md:96 says "anything other than observable", but a Simulator open with no CI is still pending. Smoke Tests, Repo Guards and Coverage were queued or running when I looked, with no failures yet. I could not verify the CI artifact numbers (54 of 61 probe-failed, 17 stale taps) from here. Before merge, the iOS Settings smoke run above must show a probe-failed open followed by a settled snapshot-route click on General at the settled y. |
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
The code now matches the earlier review (17d16c2): the docs at website/docs/docs/commands.md:96 scope the settle to a present, non-observable postOpenObservation, as in https://github.com/callstack/agent-device/blob/7f9772f/src/daemon/deferred-interaction-outcome.ts#L123. One ask remains from that review: there is still no live iOS Settings replay on 7f9772f. The first click after a probe-failed open now adds about 2.5 s and skips the direct selector tap, so we need a trace from that exact route before we can say it removes the stale tap. Please attach the Smoke Tests run for 7f9772f with per-step request logs (https://github.com/callstack/agent-device/blob/7f9772f/.github/workflows/ios.yml#L175) that show four things: the open response has timing.postOpenObservation 'probe-failed'; the next click's log has the post-open settle phase with at least two agreeing reads and goes through the snapshot route, not the direct selector route; the tap lands on General at the settled y (about 406, not 319); and the click's wall time. If the run's open reports 'observable', it did not reach this route and cannot count. Smoke Tests is still queued on this head, not failed. That job runs 01-settings.ad, which is the route this change touches, so a failure there would count against the PR. The delta also adds --debug to that job's command line. Not blocking, take or leave: the docblock at src/daemon/deferred-interaction-outcome.ts:112 names "a URL-only open, a physical device" but omits a Simulator open that reports no verdict (no bridge or no appBundleId), so "an open that reports no verdict" would cover it without naming causes. The cubic-dev-ai thread on the docs scoping is fixed at this head and can be resolved (#3376 (comment)). The cubic-dev-ai P3 thread on gating by cause does not apply, so please resolve it: the mark is gated on typed presence and not 'observable', and open-policy.ts:164 sleeps a fixed delay for every non-observable verdict, so no settled read was proven for any of them (#3376 (comment)). I did not run the tests, so the PR body's test counts and the CI artifact figures (54 of 61 probe-failed, 17 stale taps) are unverified. Before merge, the Smoke Tests run on 7f9772f must show that settled snapshot-route click on General. |
7f9772f to
770466b
Compare
|
Smoke Tests on
Caveat: both reads agreed, so the layout was already at rest here. This run proves the route, not that a 319 → 406 shift gets corrected. That case happens in about 8.5% of runs. With New head
|
|
I found no problems in the code at 770466b. The earlier findings from 7f9772f are fixed. The docs in All 21 checks pass. The Smoke job runs I read the diff but did not run the unit or integration tests. That the new tests fail on the old condition comes from reading the code, not from running them. I did not open Smoke run 38065900476. I rely on your quoted log lines for the probe failure, the two snapshot reads, Nothing else needs to change before merge. Once you are happy with that trace and cost, this is ready to merge. |
|
On the cost question: yes, I think about 1.3 s for the second read is acceptable.
The real test is CI's attempt-1 failure rate on |
… read the app (#3367) CI evidence across 30 runs of 01-settings.ad: 61 iOS opens reported 54 probe-failed, 7 observable, 0 unobservable, so the unobservable-only mark from #3356 never fired. Every stale-tap failure followed a probe-failed open. Mark the next capture for any defined verdict other than observable, mirroring the open-policy fixed-delay condition; an open with no verdict (physical device) still does not mark. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… not its cause Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
770466b to
6f93248
Compare
Summary
#3356 settled the first screen read after an iOS Simulator
openonly when the open reportedunobservable. On CI that never happens. Across 30 CI artifact bundles, 61 opens reported 54probe-failed, 7observable, and 0unobservable. All 17 stale-looking taps (General tapped at y=319 instead of the settled 406) followed aprobe-failedopen, including the original evidence run37815426076. So #3356 never fired where the failures happen.The mark now fires for any verdict other than
observable. That is the same conditionopen-policy.tsalready uses for its fixed post-open delay. An open with no verdict (a physical device) still doesn't mark.Behavior note: while a settle is pending, the direct iOS selector tap path stays off. So the first Maestro-compatible click after such an open goes through the snapshot route.
Tests: scripted fixtures whose opens report
probe-failedgain the extra settle reads. The interaction-contract and press-readiness harnesses take one settlingsnapshotright afteropen, so each scenario's own runner calls stay exactly as scripted.Docs,
commands.md, and theTAP_OUTCOME_NOT_OBSERVED_GAPcell now name the wider rule.Refs #3367, #3354.
Validation
unobservablemakes the 2 new verdict tests fail.pnpm check:affected --runpassed on770466b60(rebased).pnpm test:unit(13146 tests),test:integration:provider(232 tests), and the interaction-contract tests also pass.38065900476with per-step logs: the open reportedprobe-failed, two snapshot-route reads agreed (post_gesture_snapshot_stabilized, actionopen), and the tap landed at (201, 406).🤖 Generated with Claude Code