Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
|
[claude-fable-5-1] responding on behalf of Oskar CI note: Smoke Tests failed at |
|
This looks unproven at 8bc857c. The only run in the description is scroll + wait stable where each wait settled with captures: 2, which is the normal return path through runPostGestureStabilizationLoop that main already clears the record on — see https://github.com/callstack/agent-device/blob/8bc857c/src/daemon/deferred-interaction-outcome.ts#L356. Nothing shows a wait-deadline abort thrown out of the loop followed by a later capture in the same session, and the tests don't reach it either: the new case calls capturePostGestureStabilizedResult directly with a hand-built DOMException instead of going through wait stable. Is the iOS AX-bridge or runner capture confirmed to throw when its signal aborts, rather than just returning? If it returns instead of throwing, the loop runs to its cap and clears the record on main too, and #2885 would have a different cause. To validate, open the fixture Automation lab (or Settings) on the iOS simulator at this head, run The failing CI step is replay step 1 (launchApp), where the simulator openurl of agent-device-test-app:///automation?event=cold.start failed with COMMAND_FAILED before any capture; the diff only changes when capturePostGestureStabilizedResult clears the record, which runs inside snapshot captures, so this failure looks unrelated to the change. I did not check the author's claim that this lane also fails on main. Not blocking: the new test could exercise the real wait-stable path (runWithinWaitDeadline -> captureSelectorSnapshot -> captureSnapshot -> resolveDeferredInteractionOutcome) instead of calling capturePostGestureStabilizedResult directly with a stub exception, and the inline comment at deferred-interaction-outcome.ts:328-330 could state the consume-on-entry rule instead of narrating the issue history, but either can be taken or left. The next thing needed before merge is that live simulator evidence: a run showing a wait-deadline abort mid-loop consumes the record, and the following wait stable settles with captures >= 2 without paying for another post-gesture loop. |
A wait stable poll bounds each capture with a deadline abort. When that abort landed inside the post-gesture stabilization loop the pending record stayed armed, so every later capture on the session paid the whole loop again and wait stable kept timing out with zero captures. The record is now cleared once the loop has been entered, on settle, timeout and abort alike. Refs callstack#2885.
8bc857c to
654bb7f
Compare
|
Following up on the earlier review (654bb7f), this is still not ready to merge. The abort path that this fix targets still has no live evidence. The prior review asked for a specific run: scroll, then Not blocking: the CI shows all checks passing (or unreported) at 654bb7f; the prior run's Smoke Tests failure was in an unrelated fixture step before any capture ran, so it doesn't overlap this diff, though I haven't independently pulled current logs. No conflicts known. The next thing needed before merge is a live iPhone or Simulator run using a short |
|
[claude-fable-5-1] responding on behalf of Oskar Live run at 206d61c, iPhone 17 Pro simulator (iOS 26.5), Settings app, This branch:
The first deadline aborts the loop mid-capture and consumes the record. The second short wait already counts a plain capture, and neither later wait pays the loop. main at 3660c7f, same recipe on a second iPhone 17 Pro simulator:
On main the record survives both aborts (second short wait still captures: 0) and the first normal wait re-enters the loop, which is the post_gesture_snapshot_stabilized entry. So the leaf capture does throw on abort; a silent return would have run the loop to its cap and cleared the record on main too. Also took the comment rewording at deferred-interaction-outcome.ts:328. Left the unit test as is: the daemon owner is the seam that changed, and the live run above covers the wait-stable path end to end. |
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
|
[claude-fable-5-1] responding on behalf of Oskar CI note on c23d1b7: one of the four iOS Smoke lanes failed at step 7, |
|
Reviewed c23d1b7: no code findings, and this looks ready for human review. The change since 654bb7f only rewrites a comment. The live run you posted (#2983 (comment)) follows the recipe the earlier review asked for: on this branch the second short The one failing Smoke lane is likely unrelated. It fails at https://github.com/callstack/agent-device/blob/c23d1b7/test/integration/ios-simulator-e2e/live-automation-scenario.ts#L27 in |
thymikee
added
the
ready-for-human
Summary
Refs #2885.
wait stablebounds each capture withrunWithinWaitDeadline, which aborts the capture's signal with aTimeoutErrorat the wait's deadline. After ascroll, that capture routes throughcapturePostGestureStabilizedResult, which runs the post-gesture stability loop (up to the 3.5 s distrust cap, each poll a full simulator capture) inside what the stable loop counts as one capture. When the wait deadline aborted the loop mid-poll, the throw skippedclearPostGestureStabilization, so the pending record stayed armed and every later capture on the session paid the same loop again: the nextwait stablealso stalled withcaptures: 0, and the one after it, which is the 15-of-16 pattern in the issue.The owner now clears the record in
finally: entering the loop consumes it, on settle, timeout and abort alike. No other behavior changes; the stalled timeout still reportscaptureStalled: trueandcaptures.Touched: 1 source file, 1 test file.
Validation
Tested commit
206d61c3a(654bb7f plus a comment rewording).pnpm check:affected --run: passed.post-gesture-stabilization.test.ts: a second capture that throws aTimeoutErrormid-loop leavessession.postGestureStabilizationundefined. Removing thefinallyfails it.--json --debug:scroll down 0.75, thenwait stable 100 400twice (deadline shorter than the stabilization loop), thenwait stabletwice. First short wait: timeout,captureStalled: true,captures: 0. Second short wait:captures: 1, so the record was consumed. Both normal waits settled withcaptures: 3and nopost_gesture_*diagnostic. Same recipe on main 3660c7f: second short wait stillcaptures: 0and the first normal wait logspost_gesture_snapshot_stabilized, meaning it re-entered the loop. Full tables in the PR comments.wait stableafter a gesture. That is a second seam (the stable loop is itself a settle observer and could skip the nested loop); this PR stops the stall from repeating.