fix(ios): press the navigation bar back button and speed up the back swipe - #4508

Size Report

Startup median (7 runs, lower is better):

Apex Sentinel · ✅ Approve

Since the last review: 0 new · 0 still open · 0 no longer raised

No open findings.

2 findings below minor hidden (user_settings.min_severity).

1 new finding away from what changed since f1799d6 not shown. Comment @apex-sentinel-staging review for a full review.

Reviewed with this repository's own Sentinel configuration (b68ed4bf285a).

Earlier reviews

Thanks for this. At 32278da the back-button path has one defect that should be fixed before merge. The 22 checks pass, including the smoke back step at test/integration/ios-simulator-e2e/live-automation-scenario.ts:197, but the logs do not show whether that step took the button or the swipe path. I did not run the runner tests or a live device.

The optional button probe reads isHittable, .frame and allElementsBoundByIndex outside any issue containment (Navigation.swift:290, and again at :304). The only guard is a diff of suppressedAxSnapshotIssueCount, and record() counts only kAXError-worded issues. Two other issue types still reach super.record: an element that is gone on the next resolution (when the bar's buttons are mid-transition) and a hung query. back converts a recorded failure, so the dispatcher turns an ok response into XCTEST_RECORDED_FAILURE and drops the cached target. That covers a confirmed button pop and a verified swipe that ran after the probe returned no button. A retrying agent would then go back twice. The rule: every XCTest element read in the back-button path, both the pre-tap lookup and each post-tap confirmation read, runs inside holdingRecordedIssues (RunnerTests+TextInputProbe.swift:137). A held issue means unreadable. Before the tap, return nil and swipe. After the tap, the read proves nothing. This replaces the rejectedAxSnapshotQueryCount diff. Please add a unit test that injects a recorded issue into the probe, as litPanelWindowWalkIssueForTesting does in elementSearchRoots, and assert that back still answers from the swipe path and not XCTEST_RECORDED_FAILURE. I could not reproduce this, so it rests on how the rest of the runner guards the same reads.

Not blocking, and you can take or leave these: the button path takes its before-state with no stillness check, so a push still animating can look like a confirmed pop (two equal reads before the tap would fix it); the bar-state entry misses apps with a constant bar identifier and .minimal back titles, so a real pop could answer BACK_SWIPE_UNVERIFIED (count bars and the title, or note the limit in commands.md); the 1 s confirmation budget is checked only between reads, so it is not a hard bound; no test covers the two-candidate rule or the skipped-read branch, so a mutation to frames.first passes (a pageSheet fixture with two hittable BackButtons would cover it); cutting the after-sample delay to 0.1 s narrows the simulator still span to about 0.4 s, and the --slow-toggle retune to 0.3 s follows the new code, so name that span in commands.md; the tap-then-fallback sequence at :210 repeats the tap --synthesized arm and its .performed guard cannot fire on iOS; the visionOS hints in runtime.ts:36 and Navigation.swift:134 still say back is a left-edge swipe, and a thrown tap logs a swipe-verification error line; and the PR body gives timings without the commands, raw output and runner lines that docs/agents/device-verification.md asks for. A physical-device run is preferred, not required. If you do one, please paste an AGENT_DEVICE_RUNNER_BACK_NAVIGATION_BAR_BUTTON line with found=yes pressed=yes confirmed=yes from Settings > General, and a found=no line plus a BACK_SWIPE_VERIFICATION outcome=performed line from the Safari history swipe, with the exact commands.

The earlier bot threads on single-screen scope, the settle budget, the swipe fallbacks and the log line are fixed at this head or answered. You can resolve r4564566869, r4564566875, r4564566942, r4564566951, r4564566963, r4564566971, r4564566978, r4564636721, r4564897969, r4564897988 and r4564898010. The one on single-screen scope (r4564566875) is correct as written: that scope applies only when the window is unresolved (ADR 0025).

There are no conflicts. Before merge, please contain the lookup and confirmation reads in holdingRecordedIssues and add the test that injects a recorded issue.

Re #4508 (comment): fixed in 0d17e3e.

• Every element read in the back-button path now runs inside holdingRecordedIssues: the pre-tap lookup (navigationBarBackButtonFrames and the before-state) and each post-tap confirmation read (heldNavigationBarRead in RunnerTests+Navigation.swift). This replaces the suppressedAxSnapshotIssueCount diff.
• A held issue before the tap reads as unreadable (found=unreadable), so back swipes. A held issue after the tap proves nothing: that read never confirms, so back answers back_swipe_unverified after the 1 s budget. It never records a failure and never swipes after the tap.
• New tests inject a recorded issue through navigationBarBackReadIssueForTesting, like litPanelWindowWalkIssueForTesting. testInAppBackWithAnIssueInTheButtonLookupAnswersFromTheSwipe asserts the response is ok, with found=unreadable pressed=no. testInAppBackWithAnIssueInTheConfirmationAnswersUnverified asserts BACK_SWIPE_UNVERIFIED with the tap delivered. An escaped issue would fail either test through super.record.
• The interactive pop is disabled in the fixture, but the edge swipe still popped it on iOS 27, so testInAppBackPressesTheNavigationBarBackButton now also checks that the button path ran (pressed == true).

Validation: 23 back runner tests pass on an iOS 27.0 simulator (iPhone 17 Pro). check:xctest-selection, typecheck, lint, and focused vitest pass. Latency is unchanged: back from Settings > General took 0.57 s median over 8 runs (558–582 ms). Every run logged found=yes pressed=yes confirmed=yes, with lookup ~113 ms and confirmation ~78 ms.

I did not take the non-blocking notes in this push.

Device verification at dbf400b

iPhone 17 Pro simulator, iOS 27.0, Xcode 27.1. The CLI is this worktree's build (pnpm build), with its own state directory. The daemon built the runner from this source.

AD="node bin/agent-device.mjs --state-dir /private/tmp/ad-pr4508-state --platform ios --udid BC54AC11-CD86-4C10-8C4E-5101C773FFEB --session pr4508"

Button path: Settings > General

$AD open com.apple.Preferences --relaunch
$AD snapshot -i            # @e6 [cell] "General"
$AD press @e6 --settle
$AD back --json

{ "success": true, "data": { "action": "back", "mode": "in-app", "message": "Back" } }

Runner log (sessions/pr4508/runner.log):

AGENT_DEVICE_RUNNER_BACK_NAVIGATION_BAR_BUTTON found=yes pressed=yes confirmed=yes lookupMs=154 confirmMs=78

8 more runs of press General, then back. All exited 0. Wall time per CLI call was 0.80–0.88 s, including Node startup:

found=yes pressed=yes confirmed=yes lookupMs=157 confirmMs=77
found=yes pressed=yes confirmed=yes lookupMs=154 confirmMs=73
found=yes pressed=yes confirmed=yes lookupMs=155 confirmMs=83
found=yes pressed=yes confirmed=yes lookupMs=152 confirmMs=83
found=yes pressed=yes confirmed=yes lookupMs=152 confirmMs=80
found=yes pressed=yes confirmed=yes lookupMs=150 confirmMs=79
found=yes pressed=yes confirmed=yes lookupMs=162 confirmMs=76
found=yes pressed=yes confirmed=yes lookupMs=151 confirmMs=82

The stillness check costs one more bar read. The lookup median went from about 113 ms at 0d17e3e to 154 ms. Confirmation did not change.

Swipe path: Safari history (no navigation bar back button)

$AD open com.apple.mobilesafari https://example.com
$AD fill @e35 "https://www.iana.org/help/example-domains"   # address bar
$AD keyboard return
$AD back

Back
AGENT_DEVICE_RUNNER_BACK_NAVIGATION_BAR_BUTTON found=no pressed=no confirmed=- lookupMs=44 confirmMs=-
AGENT_DEVICE_RUNNER_BACK_SWIPE_VERIFICATION outcome=performed code=- changed=yes stableMs=418 justBeforeToAfterMs=314 gestureMs=117 maxCaptureMs=95 settleSamples=4 settled=true

Cleanup: close --shutdown on the session, and daemon stop --state-dir /private/tmp/ad-pr4508-state.

Runner tests (same simulator)

All 30 methods in RunnerTests+NavigationTests.swift, via xcodebuild test-without-building -xctestrun … -only-testing:AgentDeviceRunnerUITests/RunnerTests/<each>, built with AGENT_DEVICE_XCUITEST_INCLUDE_UNIT_TESTS=1:

Executed 30 tests, with 0 failures (0 unexpected) in 136.485 (136.500) seconds

Mutation check. I removed the ambiguous arm, so two candidates take frames[0], and forced the readability guard true. Both new tests fail:

testInAppBackPressesNoneOfTwoNavigationBarBackButtons : XCTAssertEqual failed: ("yes") is not equal to ("ambiguous")
testInAppBackSkipsTheButtonLookupOnAPenalizedChannel : XCTAssertEqual failed: ("yes") is not equal to ("skipped")
Executed 2 tests, with 5 failures (0 unexpected)

Thanks. I applied every non-blocking note in dbf400b.

1. Stillness before the tap. The lookup reads the bar state, then the hittable back buttons, then the state again. It taps only when two reads in a row agree and contain the button's frame. Otherwise it retries every 0.05 s. If the bar is still moving after 1 s, it swipes with found=moving, and nothing has been sent. Cost: one more bar read, about 40 ms (lookup median 113 → 154 ms on Settings > General).
2. Constant identifier and .minimal titles. The compared state now holds, per bar and in bar order, the identifier, the static-text labels (the title), and the BackButton label and frame. The bar count is part of it. commands.md and RUNNER_PROTOCOL.md name the case that still answers unverified: same title, fixed identifier, untitled back button.
3. 1 s budget. I did not make it a hard bound. XCTest has no per-query deadline the runner could use without a different read path. Reads now start only within the budget, checked before each bar's snapshot and before each button read. The docs state the bound: a query in flight finishes, or ends at XCTest's own query timeout, which back treats as a hung query.
4. Two-candidate and skipped-read tests. A new --two-stacks fixture puts two pushed stacks on screen, so two hittable BackButtons are visible (more deterministic than a pageSheet, where the lower bar sits behind the dimming view). A second test uses a penalized channel. With the ambiguous arm removed (so frames[0] is taken) and the readability guard forced true, both tests fail ("yes" is not equal to "ambiguous" / "skipped").
5. Still span. commands.md names it: 0.2 s contact allowance, 0.1 s after-sample delay and the slowest capture, about 0.4 s on a simulator. It also says that the runner's test screen repaints every 0.3 s for this reason. The constants are unchanged.
6. Tap fallback. I kept the XCTest fallback arm, because a refused synthesis on a readable tree still reaches it. Its .performed guard was dead: on iOS with a readable tree, tapAt runs the XCTest tap or throws. I removed the guard and left the behaviour as it was.
7. visionOS hints. Both now say "visionOS has no back command. Press the visible back control by its ref." A thrown tap logs only the BACK_NAVIGATION_BAR_BUTTON line. The swipe-verification error line now appears only for a throw from context resolution or the swipe.
8. Evidence. The commands, raw output, and both runner lines from a simulator are in fix(ios): press the navigation bar back button and speed up the back swipe #4508 (comment): found=yes pressed=yes confirmed=yes on Settings > General, and found=no plus outcome=performed on Safari history. The PR body links them. No physical-device run.

Also in this push, from two new Cubic threads:

• Hung query. Per record(_:), the back reads now record a hung query again instead of holding it, and send nothing more. Before the tap that answers BACK_SWIPE_NOT_DISPATCHED, and after it BACK_SWIPE_UNVERIFIED. Two new tests cover it.
• Lookup-issue test. It now asserts the swipe path, not a swipe outcome, because iOS 27 pops the fixture on the edge swipe.

All 30 navigation runner tests pass on an iPhone 17 Pro simulator (iOS 27.0). check:xctest-selection, typecheck, lint, and focused vitest pass. The old bot threads you listed were already resolved.

@apex-sentinel-staging review

This PR is ready at 3728f47: I found no new problems, and the earlier findings (#4508 (comment)) are now fixed. A hung query is re-recorded and no longer held, the lookup-issue test asserts the trace found=unreadable / pressed=false, and snapshot frames are offset by the window origin before the live-frame compare. Not blocking, and you can take or leave these: NavigationBarBackReadPhase is only read by the unit-test injection filter (RunnerTests+Navigation.swift:418), so its doc comment describes caller behaviour it does not control; the hung re-record could live in holdingRecordedIssues so the other held reads obey the record() contract too (RunnerTests+Navigation.swift:444); and the iPad evidence predates e946cb3, so a placed (non-full-screen) iPad Settings window run showing AGENT_DEVICE_RUNNER_BACK_NAVIGATION_BAR_BUTTON found=yes pressed=yes confirmed=yes would close that gap.

The three earlier review threads on the hung query, the lookup-issue test and the snapshot frame offset are fixed at this commit, so you can resolve them: #4508 (comment), #4508 (comment), #4508 (comment).

I did not run the runner tests or a live device; the test and mutation results are from your posted output. 21 checks pass, and the only one not finished is an external review check, which exercises no build or device route. No conflicts are known. Once that check finishes, nothing else stands in the way of merging.

@apex-sentinel-staging review

I found no correctness problems at cee9941, but the iOS checks have not finished, so I can't call this ready yet. The earlier review at 3728f47 was clean, and the changes since then fix the three open inline threads: the loop now does one read per round before sleepFor, confirmation compares only back button labels, and bar titles are no longer read. Smoke Tests (ios), Numeric Ranges, Coverage and one external review check were still running and none had failed. The iOS smoke job runs the changed back route through the runner tests, so its result is the validation still needed. I did not run the runner tests or a device. The simulator pass and the Settings > General result come from the PR body. The iPad placed-window evidence is still from e946cb3, and the new label-only confirmation was not re-run there.

The three inline threads are fixed at this head and can be resolved: #4508 (comment), #4508 (comment) and #4508 (comment).

Not blocking, and you can take or leave these: (1) TimedNavigationBarBackState and the timing clause guard a case the loop cannot reach, since every compared pair is already one sleepFor apart; (2) navigationBarBackFrameTolerance has no run showing a frame mismatch, and the placed iPad run matched exactly; (3) with label-only confirmation, a real pop on a stack of three or more screens where the outgoing and incoming back buttons share a label would report back_swipe_unverified after the full 1 s, and I did not measure whether XCTest reports the same label for UIKit's long-title "Back" substitution; (4) the back_swipe_unverified reason, the commands.md bullet and the runner message still say "the navigation bar did not change", where "the navigation bar's back buttons did not change within 1 s" is now accurate.

Could you delete the timed-state type, the timing clause and the tolerance (and its test) unless you can cite a run where exact equality failed? That would remove about 30 production lines and keep the two real fixes, the label-only confirmation and the one-read-per-round loop. After that, the remaining step before merge is a passing iOS smoke run on the final head.

Applied the simplification at f443d78:

• (1) Removed TimedNavigationBarBackState and the timing clause. The still check is now current == previous inside the one-read-per-round loop, with sleepFor between the compared reads. The timing-only test is gone. Its moved-button assertion now sits in the popped test (XCTAssertNotEqual(moved, detail)).
• (2) Removed navigationBarBackFrameTolerance and its test. The match uses exact CGRect equality after the screen-space conversion. The placed-window test is kept.
• Label-only confirmation is unchanged.
• (4) The back_swipe_unverified help text, the commands.md bullet, the runner message and RUNNER_PROTOCOL.md now say the navigation bar's back buttons did not change within 1 s. commands.md also notes the blind spot from (3): a pop on a stack of three or more screens whose outgoing and incoming back buttons share a label answers back_swipe_unverified after the full 1 s.
• The swipe constants are untouched.

Non-test production lines vs main go from +455/−67 to +430/−67.

Validation: 29 back runner tests pass on an iPhone 17 Pro simulator (iOS 27.0). Settings > General gives found=yes pressed=yes confirmed=yes in 4 of 4 runs. The macOS runner test build succeeds. Typecheck, lint, xctest-selection and focused vitest pass. I did not re-run iPad. The three inline threads were already resolved.

The iOS smoke failure in run 43488040274 (smoke:automation-input, "return from automation route") was a real pop reported as unverified. The runner log shows that the confirmation reads after the tap were fast (~90 ms). The third read listed the navigation bar at the moment it was leaving. Its snapshot() then spent about 2 s in XCTest's element-resolution retries and ended in a held issue ("No matches found for NavigationBar"). By then the 1 s budget was gone, so no read saw the settled Settings root, which has no bar.

Fix in 1154091: after the 1 s budget, the confirmation always takes one more read with no deadline, so it always reads the screen after the pop. Zero bars or zero back buttons already counts as a change. Confirmation still uses labels only, and a hung query still answers at once. A new runner test replays this: two slow reads that fail inside the budget, then the pop must be confirmed. It fails without the final read. The 22 back runner tests and the local iOS smoke tier pass on an iPhone 17 Pro simulator.

iPad placed-window verification at 40c2f69

This run uses the label-only confirmation. 40c2f69 changes only a doc comment from f1799d6, and the daemon built the runner from this source.

Setup: iPad Pro 11-inch (M5) simulator, iOS 27.0, Xcode 27.1. Multitasking is set to Windowed Apps. The CLI is this worktree's build (pnpm build), with its own state directory.

AD="node bin/agent-device.mjs --state-dir /private/tmp/ad-pr4508-ipad-state --platform ios --udid 9D075F91-A796-49ED-8C83-947137E611B6 --session pr4508ipad"
$AD open com.apple.Preferences --relaunch
$AD press @e10 --settle                           # Multitasking & Gestures
# Shrink Settings from full screen into a placed window by dragging the bottom-right resize grip:
$AD gesture pan 816 1192 -200 -350 1200
$AD back --json

After the drag, the Settings window sits at about x 197–637, y 230–865 on the 834×1210 screen, with the wallpaper and Dock visible around it. Its back button is in the window's own bar.

{ "success": true, "data": { "action": "back", "mode": "in-app", "message": "Back" } }

Then I ran 4 more rounds of press 'label="General"' --settle then back, all in the same placed window. All exited 0. Runner log (sessions/pr4508ipad/runner.log), all 5 runs:

AGENT_DEVICE_RUNNER_BACK_NAVIGATION_BAR_BUTTON found=yes pressed=yes confirmed=yes lookupMs=279 confirmMs=88
AGENT_DEVICE_RUNNER_BACK_NAVIGATION_BAR_BUTTON found=yes pressed=yes confirmed=yes lookupMs=233 confirmMs=67
AGENT_DEVICE_RUNNER_BACK_NAVIGATION_BAR_BUTTON found=yes pressed=yes confirmed=yes lookupMs=236 confirmMs=68
AGENT_DEVICE_RUNNER_BACK_NAVIGATION_BAR_BUTTON found=yes pressed=yes confirmed=yes lookupMs=252 confirmMs=67
AGENT_DEVICE_RUNNER_BACK_NAVIGATION_BAR_BUTTON found=yes pressed=yes confirmed=yes lookupMs=233 confirmMs=68

Cleanup: close --shutdown on the session, then daemon stop --state-dir /private/tmp/ad-pr4508-ipad-state.

Re #4508 (comment), at 40c2f69:

1. NavigationBarBackReadPhase doc comment. Rewritten. It now says only what the type is: the phase a bar read belongs to (lookup, confirmation, or final confirmation), passed to the test read hook. The code does not change.
2. Move the hung re-record into holdingRecordedIssues. Not done in this PR. Moving it would change issue containment for every runner command that uses holdingRecordedIssues, and that is outside this PR's scope. The maintainer also asked for a smaller delta. I can open a follow-up issue for it if you want one.
3. iPad evidence. I re-ran the placed (non-full-screen) iPad Settings window on an iPad Pro 11-inch simulator, with Windowed Apps on and Settings shrunk into a window. This used this head's label-only confirmation. All 5 of 5 runs gave AGENT_DEVICE_RUNNER_BACK_NAVIGATION_BAR_BUTTON found=yes pressed=yes confirmed=yes. The exact commands are in fix(ios): press the navigation bar back button and speed up the back swipe #4508 (comment), and the PR body's iPad line now points there.

Validation: check:xctest-selection passes, the macOS runner test build succeeds (TEST BUILD SUCCEEDED), and lint passes. I did not change the swipe constants or the confirmation logic.

This PR is ready at f1799d6. The trims requested in the earlier review (#4508 (comment)) landed, and I found no new problems in the code. The newer head 40c2f69 only rewrites the NavigationBarBackReadPhase doc comment to describe it as the read hook's phase, which is what I would have suggested, and changes no code.

All 22 checks pass on f1799d6. Smoke Tests (ios) exercises the changed back route, and it is the job that failed on the run before the earlier fix. No conflicts. No further action is needed before merge.

The three earlier review threads are fixed at this head, so you can resolve them: the phase-gated test (#4508 (comment)), the RUNNER_PROTOCOL.md wording on the final read (#4508 (comment)), and the "within 1 s or on one more read" wording in the docs, help and runner message (#4508 (comment)).

What I did not check: I did not run the runner unit tests or a device, so the claim that the new test fails without the final read comes from reading the code and your report. I also did not open the CI runner log for the ~2 s snapshot figure. The exact frame equality that replaced the 0.5 pt tolerance was validated on a placed iPad window only at e946cb3, not at this head. A mismatch there would fall back to the swipe after 1 s and would not misreport.