Skip to content

refactor: make iOS synthesized gesture policy explicit#1152

Merged
thymikee merged 8 commits into
mainfrom
refactor/ios-synthesized-gesture-policy
Jul 8, 2026
Merged

refactor: make iOS synthesized gesture policy explicit#1152
thymikee merged 8 commits into
mainfrom
refactor/ios-synthesized-gesture-policy

Conversation

@thymikee

@thymikee thymikee commented Jul 8, 2026

Copy link
Copy Markdown
Member

Summary

Make iOS synthesized gesture policy explicit in the runner with command/step-local contexts for AX health, frame source, keyboard probing, and fallback rules.

Default iOS scroll remains private-synthesis-only on AX unknown/unavailable screens. Explicit synthesized drag and sequence steps keep first-command keyboard avoidance and coordinate fallback while AX health is unknown, stop those AX-dependent paths once AX is stamped unavailable, and use screenshot-size coordinate frames instead of app.frame so screenshot coordinates are not offset on non-fullscreen layouts. The TS policy checks now reuse a shared runner-source assertion helper.

Validation

  • pnpm format
  • pnpm exec vitest run src/contracts/__tests__/synthesized-gesture-policy.test.ts src/contracts/__tests__/interaction-guarantees.test.ts
  • pnpm check:quick
  • pnpm build:xcuitest
  • AGENT_DEVICE_XCUITEST_INCLUDE_UNIT_TESTS=1 pnpm build:xcuitest

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.6 MB 1.6 MB +161 B
JS gzip 506.3 kB 506.4 kB +68 B
npm tarball 606.8 kB 608.3 kB +1.5 kB
npm unpacked 2.1 MB 2.2 MB +8.1 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 28.0 ms 27.4 ms -0.6 ms
CLI --help 54.5 ms 54.0 ms -0.5 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/cli-help.js +161 B +68 B

@thymikee

thymikee commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

Coverage is currently failing on CI for run 28941828847 / job 85865607515, while the workflow still has queued iOS/macOS smoke jobs. GitHub is not exposing the failed job log yet because the workflow is still in progress. Please inspect the Coverage logs when the run completes, address the failure, and reply with what changed and what validation remains blocked.

@thymikee

thymikee commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

Review — refactor: make iOS synthesized gesture policy explicit

Nice consolidation — moving the AX-health / keyboard / fallback rules into one explicit policy object (plus the JSON manifest + vitest/XCTest cross-check) makes the previously-implicit iOS gesture behavior legible, and the scroll privateSynthesisRequired guarantee is preserved and now asserted. A few things worth a look, mostly around the new default runnerAccessibilityHealth = .unknown and the healthy-AX frame source. These are behavior changes on paths the PR's own new smoke-check help text (press … / swipe …) actively encourages as first commands.

Correctness / behavior

  1. RunnerTests+Interaction.swift (allowsKeyboardProbe, ~L1175 & L1191) — keyboard avoidance silently turns OFF before the first snapshot.
    Old shouldProbeKeyboardForSynthesizedGesture() was !lastSnapshotHadAccessibilityUnavailable, which defaults to true (probe on). New SynthesizedKeyboardPolicy.whenAccessibilityHealthy.allowsProbe(.unknown) is false. So a synthesized swipe/scroll issued as the first command of a session (health .unknown, no snapshot yet — exactly the gesture smoke-check flow this PR documents) no longer reflows its start/end points away from the on-screen keyboard; the drag can run across keyboard keys and be swallowed. Not mentioned in the PR description — looks like an unintended narrowing of the old default.

  2. RunnerTests+Interaction.swift (synthesizedCoordinateContext / finiteSynthesizedReferenceFrame, ~L1123–1131) — healthy AX now maps coordinates against app.frame instead of the screenshot size.
    Old synthesizedGestureReferenceFrame hardcoded appFrame: .zero and always resolved to the screenshot image size (origin 0,0). New code passes appFrame = (health == .healthy ? app.frame : .zero) and prefers a usable app.frame. Since nativeSynthesizedPoint computes localX = x - frame.minX, when app.frame is not full-screen / has a non-zero origin (iPad Split View, Stage Manager, Slide Over), a coordinate the agent picked from a full-screen screenshot lands offset by the frame origin. This changes the supposedly-unchanged "healthy" path; on a normal full-screen iPhone app it's a no-op (origin 0,0), so the blast radius is multitasking/non-fullscreen layouts.

  3. RunnerTests+CommandExecution.swift (synthesized drag, ~L1695) & RunnerTests+SequenceExecution.swift (sequence tap, ~L189) — no XCUICoordinate fallback in the .unknown pre-snapshot state.
    Gating drag/sequence fallback to healthy AX is stated intent ("fall back only when AX is healthy"), but note .unknown (before any snapshot) is neither healthy nor unavailable, so a first-command swipe/synthesized-drag whose private synthesis fails now returns a hard unsupported error where main fell through to the XCUICoordinate path and could still move content. Worth confirming this interaction with the new "run press/swipe as a smoke check" guidance is intended — a snapshot-first ordering would sidestep it.

Cleanup (non-blocking)

  1. RunnerTests+Interaction.swift (~L1095 & L1114) — dead overload + unused params. The keyboardPolicy:/fallbackPolicy: overload has no direct callers (every site uses the policy: form), and providedFrame / accessibilityHealth / fallbackBounds / fallbackScreenshotSize are never passed by any caller. Collapsing to a single synthesizedCoordinateContext(app:policy:) that reads policy.keyboardPolicy/.fallbackPolicy inline removes a forwarding method and a duplicated 7-param signature.

  2. src/contracts/__tests__/synthesized-gesture-policy.test.ts (~L99) — duplicated runner-symbol check. The via.split('#') → path.join(RUNNER_SOURCES_DIR, …) → existsSync → readFileSync().includes(symbol) verifier (plus the PROJECT_ROOT/RUNNER_SOURCES_DIR constants) is copy-pasted from interaction-guarantees.test.ts. Worth extracting a shared helper so the two can't drift.


Note: I checked the ".scroll guard returns nil on non-iOS" path — it's not a real regression. The scroll interaction routes macOS → desktopScroll and tvOS → remote press in runAppleScroll, so the runner's .scroll case (and its now-dead #else branch) is only reached on iOS, where the context resolves fine.

@thymikee

thymikee commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

Coverage logs are available now. The failing test is:\n\ntest/integration/provider-scenarios/settle-observation.test.ts > Provider-backed integration press --settle returns the settled diff and fresh refs\n\nAssertion at line 139: settle.settled was false, expected true. The run otherwise shows 412 files / 3529 tests passing, so this looks like a focused settle-observation regression or test expectation mismatch rather than a broad coverage failure. Please address that specific failure along with the review findings above before re-review.

@thymikee

thymikee commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

CI is green again at 7deac38, and the coverage failure looks addressed by the provider timeout hardening. The latest commit does not address the open correctness review items from the prior pass, though: the policy still disables keyboard probing while AX health is .unknown, still uses app.frame as the healthy-AX reference frame, and still withholds XCTest-coordinate fallback before a synthesized drag/sequence context resolves.

Keeping this off ready-for-human for now. Please address those findings or reply with the intended ordering/rationale plus any live device evidence so I can re-review.

@thymikee thymikee force-pushed the refactor/ios-synthesized-gesture-policy branch from 7deac38 to 1cbd4f3 Compare July 8, 2026 13:31
@thymikee

thymikee commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

Re-reviewed updated head 1cbd4f3 after the first-command synthesized drag fix. The previous blockers are addressed: drag and sequence now keep keyboard probing and XCTest-coordinate fallback available while AX health is unknown, app.frame is no longer used as the synthesized coordinate frame source, and the runner-source symbol verifier is shared. CI is green across 21/21 checks, including iOS smoke and Swift runner jobs. No actionable blockers found.

Residual risk: I did not add local live device verification beyond reviewing the PR validation notes and green platform smoke checks.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 8, 2026
@thymikee

thymikee commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

New head b3a7ba9 is still running platform smoke checks, so I removed the stale ready-for-human label for now. I will re-review/relabel once the current run is fully clean.

@thymikee thymikee removed the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 8, 2026
@thymikee

thymikee commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

Re-reviewed updated head 6f31b39 after the synthesized policy simplification commits. The new delta keeps the prior fixes intact: synthesized command contexts use screenshot dimensions only, first-command drag/sequence keyboard probing and coordinate fallback remain available while AX health is unknown, and sequence synthesized tap/drag now share one context only because their policy values are equal and covered by unit tests. CI is green across 21/21 checks. No actionable blockers found.

Residual risk remains live-device depth: I did not run additional local simulator/device verification beyond the green platform smoke checks.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 8, 2026
@thymikee

thymikee commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

Re-checked latest head 95ae6cb. The only delta since the prior ready review is a comment correction in RunnerTests+CommandExecution.swift clarifying scroll vs explicit synthesized-drag fallback policy; implementation/tests are unchanged and CI is green across 21/21 checks. Keeping ready-for-human.

@thymikee

thymikee commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

Re-checked current head abf3af8. The latest delta intentionally keeps synthesized gesture policy runner-local: the JSON manifest/TS mirror test is removed, while the Swift unit tests and ADR 0011 now carry the policy claims. The behavior reviewed previously is unchanged: screenshot-only coordinate frames, scroll private-synthesis-only fallback, and explicit synthesized drag/sequence fallback while AX is not known unavailable. CI is green across 21/21 checks, so I am keeping ready-for-human.

@thymikee thymikee merged commit b91eaad into main Jul 8, 2026
21 checks passed
@thymikee thymikee deleted the refactor/ios-synthesized-gesture-policy branch July 8, 2026 15:15
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-08 15:15 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.

1 participant