Skip to content

feat: add gesture command coverage#576

Open
thymikee wants to merge 8 commits into
mainfrom
codex/gesture-commands-testbed
Open

feat: add gesture command coverage#576
thymikee wants to merge 8 commits into
mainfrom
codex/gesture-commands-testbed

Conversation

@thymikee
Copy link
Copy Markdown
Member

@thymikee thymikee commented May 21, 2026

Summary

Adds end-to-end gesture support and a React Native Gesture Handler test bed for pan, pinch, rotate, and fling gestures.

Details:

  • Uses the canonical CLI/replay surface gesture <pan|fling|pinch|rotate>.
  • Removes public top-level aliases for the newly added pan, fling, and rotate-gesture commands; existing pinch compatibility remains internal and undocumented.
  • Keeps provider/progress accounting and user-facing errors aligned with the gesture command group.
  • Adds iOS runner rotate gesture execution, including opposite-direction rotation handling.
  • Routes iOS pan/fling through the existing runner drag command instead of adding a separate runner fling protocol command.
  • Adds the test-app Gesture lab with the React logo image and live x/y/scale/rotation/fling metrics.
  • Adds replay, integration, unit, SkillGym, CLI help, and docs coverage for the gesture command group.
  • Related follow-up for runner overlays and raw recordings: Fix iOS simulator recording overlays so gesture commands can run during recording #574

Validation

Verified earlier on iPhone 17 with Expo Go on Metro port 8082. The recorded proof sequence completed pinch in/out, pan left/right/up/down, rotate 145 degrees, rotate -215 degrees, and fling left/right/up/down. Final app metrics after the recorded run: x 72, y 56, scale 0.60, rotate -69, fling 10.

Current checks:

  • pnpm format
  • pnpm check:quick
  • pnpm check:fallow --base origin/main
  • pnpm test:integration:progress:check
  • pnpm test:smoke (4 passed, 2 loopback-listener skips in this environment)
  • pnpm exec vitest run src/core/__tests__/dispatch-interactions.test.ts src/core/__tests__/dispatch-pinch.test.ts src/utils/__tests__/args.test.ts test/integration/provider-scenarios/ios-lifecycle.test.ts test/integration/provider-scenarios/android-lifecycle.test.ts
  • pnpm exec vitest run src/utils/__tests__/args.test.ts src/replay/__tests__/script.test.ts src/core/__tests__/capabilities.test.ts src/core/__tests__/dispatch-interactions.test.ts test/integration/provider-scenarios/ios-lifecycle.test.ts test/integration/provider-scenarios/android-lifecycle.test.ts

pnpm check:unit was also rerun. In sandbox it hit environment restrictions (listen EPERM, Swift/tooling failures). Escalated, it reduced to one unrelated failure in src/utils/__tests__/video.test.ts where the MP4 fallback assertion returned false.

Proof video: /private/tmp/agent-device-full-gesture-video/full-gesture-sequence.mp4

Touched 40 files; scope stayed within gesture command support, runner execution, test-app coverage, and command/docs guidance.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 21, 2026

PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://callstackincubator.github.io/agent-device/pr-preview/pr-576/

Built to branch gh-pages at 2026-05-22 11:58 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 053d003033

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +1073 to +1077
let target = interactionRoot(app: app)
switch direction {
case "up":
target.swipeUp()
case "down":
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Execute fling using requested coordinates on iOS

This implementation ignores the command’s x/y/x2/y2 inputs and performs a generic swipe*() on the interaction root, so fling <direction> <x> <y> [distance] does not actually start from the requested point on iOS. In screens with multiple gesture regions (for example a carousel inside a scroll view), the fling can target the wrong container or trigger unrelated navigation even though the caller supplied explicit coordinates.

Useful? React with 👍 / 👎.

Comment on lines +1335 to +1339
private func performCoordinateRotateGesture(app: XCUIApplication, degrees: Double, x: Double?, y: Double?, velocity: Double) -> RunnerInteractionOutcome {
#if os(iOS)
let target = app.windows.firstMatch.exists ? app.windows.firstMatch : app
let radians = CGFloat(degrees * .pi / 180.0)
target.rotate(radians, withVelocity: CGFloat(velocity))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply rotate-gesture center coordinates in runner

The rotate-gesture command validates and forwards optional center coordinates, but this code never uses x/y and always rotates the top window element. As a result, callers cannot control the gesture center, so rotations intended for off-center targets can affect the wrong UI region while still reporting success.

Useful? React with 👍 / 👎.

Comment thread src/core/capabilities.ts
Comment on lines +105 to +108
fling: {
apple: { simulator: true, device: true },
android: { emulator: true, device: true, unknown: true },
linux: LINUX_NONE,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restrict macOS fling capability to supported platforms

The capability matrix marks fling as supported for all Apple device kinds, which includes macOS sessions, but the runner implementation explicitly returns unsupported on macOS. This causes command admission to report support and then fail later at execution time, producing avoidable runtime errors and inconsistent platform capability behavior.

Useful? React with 👍 / 👎.

@thymikee thymikee force-pushed the codex/gesture-commands-testbed branch from 053d003 to 5ed3c06 Compare May 21, 2026 12:08
@thymikee thymikee force-pushed the codex/gesture-commands-testbed branch from 5ed3c06 to 2d8d8dc Compare May 21, 2026 12:13
@thymikee thymikee force-pushed the codex/gesture-commands-testbed branch from b62ccef to 222f753 Compare May 21, 2026 13:20
@thymikee thymikee force-pushed the codex/gesture-commands-testbed branch from 222f753 to 9b592f6 Compare May 21, 2026 13:37
Copy link
Copy Markdown
Member Author

Code review

Overview

Adds a unified gesture <pan|fling|pinch|rotate|transform> CLI/replay surface and removes the standalone top-level pinch command (now gesture pinch, with pinch kept as an internal/undocumented literal). It wires new interactor methods (pan, fling, pinch, rotateGesture, transformGesture) through Android (instrumentation-based multi-touch helper + provider-native touch injection), iOS (XCUITest runner rotateGesture/transformGesture; pan/fling routed through the existing drag), and Linux (unsupported stubs). Plus a new Android helper APK + build/package scripts, a React Native Gesture Handler test bed (GestureLab), and broad test/doc/SkillGym coverage.

The architecture is clean and consistent with existing conventions — no confirmed correctness bugs in the dispatch/parse/capability layers. A few cross-platform semantic divergences are worth confirming before merge.

Findings

Issues to verify

1 — iOS pan vs fling pre-drag hold duration looks inverted. (src/platforms/ios/interactions.ts)

  • pan passes durationMs: iosPanHoldDurationMs(durationMs), and iosPanHoldDurationMs is Math.min(durationMs ?? 500, 16) — so the pre-drag hold is always ≤16ms regardless of the requested duration.
  • fling passes durationMs straight through (default 50 from dispatch), which the Swift runner clamps to [16ms, 10s], so a fling holds ~50ms.
  • Net: a deliberate pan gets a shorter press-hold than a fast fling. Since press(forDuration:thenDragTo:) hold time influences XCUITest's inertia/velocity interpretation, this is backwards from intent. Worth confirming on-device that fling actually produces more momentum than pan. (Confirmed at the TS level by reading the file; the on-device effect should be verified.)

2 — gesture pan docs claim duration is "preserved," but iOS discards it. website/docs/docs/commands.md says pan accepts x y dx dy [durationMs] "where the requested duration should be preserved." True on Android (swipeAndroid forwards duration) but not iOS, where iosPanHoldDurationMs caps it to 16ms (it's a hold, not travel time). Either make the doc platform-specific or note the iOS limitation explicitly (as already done for gesture transform).

3 — rotateGesture velocity/degrees sign divergence across platforms.

  • Android (multitouch-helper.ts, rotateGestureAndroid): degrees = Math.abs(options.degrees) * Math.sign(velocity) — so degrees: 35, velocity: -1 rotates −35°.
  • iOS (performCoordinateRotateGesture): rotates +35° with velocity: -1.
    So an explicit velocity whose sign contradicts degrees yields opposite rotation directions per platform. The default path (velocity derived from degrees sign) is consistent; only the explicit-contradicting case diverges. Consider documenting that velocity sign should match degrees, or normalizing one platform to match the other.

4 — rotateGesture client serialization can emit the string "undefined" when only one of x/y is provided. (src/client.ts, rotateGesture center handling) The center is included when options.x !== undefined || options.y !== undefined, then emits [String(options.x), String(options.y)]. If only x is set, y becomes "undefined", which parseOptionalGestureCenter parses as NaN and rejects with "requires finite x and y" rather than the clearer "requires both x and y." The type makes both optional, so this state is reachable — minor UX papercut.

5 — rotateGestureAndroid doesn't guard velocity === 0. Math.sign(0) === 0 would zero out the degrees. The public CLI/daemon path validates velocity ≠ 0 in parseRotateGestureParams, but rotateGestureAndroid is a public export called directly in tests; a defensive guard is cheap.

6 — iOS transformGesture is three sequential gestures, not one continuous touch. The Swift transformGesture does dragpinchrotate in sequence (lifting fingers between), while the Android helper does a single combined two-pointer motion. This is documented (the iOS doc note says to verify metrics), but reviewers should be aware the semantics differ substantially (pinch/rotate centers won't track the pan offset). Also within the iOS composite, pinch targets gestureElement(...) (smallest element containing the point) while rotate targets app.windows.firstMatch — inconsistent targets within one gesture.

7 — Unrelated examples/test-app/pnpm-lock.yaml downgrades. The lockfile diff removes the overrides block and downgrades postcss 8.5.12 → 8.4.49 and uuid 14.0.0 → 7.0.3 (re-introducing a deprecated uuid). Looks like an incidental side effect of adding react-native-gesture-handler / re-resolving rather than intentional. Worth confirming it's expected. The root lockfile is untouched, so impact is limited to the example app.

Nits

  • website/docs/docs/client-api.md omits transformGesture() from the client.interactions.* list, though pan/fling/rotateGesture were added — incomplete given the method is public.
  • gesture schema uses allowedFlags: []; confirm selection/global flags consumed by buildSelectionOptions aren't unintentionally stripped (other interaction commands list real allowedFlags). skipCapabilityCheck: true is appropriate since per-subcommand capability is enforced downstream.
  • The gesture default-subcommand error string ("requires one of: pan, fling, pinch, rotate, transform") is duplicated between src/cli/commands/generic.ts and request-generic-dispatch.ts — two sources of truth.

Test coverage

Strong and the new paths are genuinely exercised: dispatch-interactions.test.ts (pan delta+duration, fling direction→drag conversion + defaults, pinch, rotate velocity-sign default, transform on Android + iOS sim), multitouch-helper.test.ts (payload encoding, output parsing, provider-native preference over adb, install caching), iOS interactions index.test.ts (pan→drag 16ms cap), runner-client.test.ts (rotate/transform fixtures + exhaustive command list), args.test.ts/script.test.ts (CLI/replay parsing), capabilities.test.ts (new matrices), plus Android/iOS integration lifecycle coverage.

Gaps:

  • No end-to-end test of the Android am instrument helper fallback wiring (resolveAndroidMultiTouchHelperArtifactensureAndroidMultiTouchHelper → run); only runAndroidMultiTouchHelperGesture is unit-tested in isolation.
  • The iOS hold-duration behavior (fix: skill should work, even if the npm package is not installed #1) is locked in by the test asserting durationMs: 16 for pan — so the test encodes current behavior and won't catch the semantic concern.
  • No automated coverage of the Java instrumentation math or the Android build/package scripts (expected — needs an Android SDK).

Summary

Solid, well-structured, thoroughly tested PR with a clean unified gesture surface and a sensible provider-native-then-helper Android strategy. No confirmed correctness bugs in dispatch/parse/capability. Before merge I'd ask the author to confirm: the iOS pan/fling hold-duration inversion (#1) and the related pan duration-preservation doc claim (#2), the cross-platform rotate velocity-sign divergence (#3), and whether the test-app lockfile downgrades (#7) are intentional. The rest are minor doc/UX nits.

Reviewed with Claude Code.


Generated by Claude Code

@thymikee
Copy link
Copy Markdown
Member Author

Addressed the review feedback in cdd8119:

  • iOS pan/fling now use distinct XCTest drag hold defaults: pan preserves requested hold duration, fling defaults short.
  • Rotate direction is normalized to the sign of degrees; velocity is treated as speed, avoiding Android/iOS sign divergence.
  • Client rotateGesture now rejects partial centers before sending any "undefined" positional values.
  • Android rotateGesture defensively rejects zero velocity before provider/helper dispatch.
  • Restored the test-app lockfile overrides by regenerating the lockfile; the accidental postcss/uuid downgrade diff is gone.
  • Added transformGesture to client API docs and centralized the gesture subcommand error string.

Validation: pnpm exec vitest run src/tests/client.test.ts src/core/tests/dispatch-interactions.test.ts src/platforms/android/tests/multitouch-helper.test.ts src/platforms/ios/tests/index.test.ts src/utils/tests/args.test.ts; pnpm check:unit outside sandbox; pnpm check:quick.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant