feat!: remove deprecated gesture duration and rotate velocity inputs (#1218, #1216) - #1315
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
…dation, migrate linux-desktop swipe test to pan Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Review at exact head
All current checks are green, but they do not exercise the execution-profile distinction. |
…ernal seam Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Thanks for the review.
All 21 CI checks are green at the latest commit. |
|
The prior implementation findings are fixed, but this head is not ready yet:
|
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Thanks for the follow-up.
All 21 per-PR checks are still green on the latest commit. |
|
Verified the requested live iOS
This supplies the requested live XCTest delivery-path evidence for the Maestro-only |
|
Thank you for running the live iOS evidence. Confirms the Maestro-only |
|
The new live run proves the exact-head iOS Maestro route can submit the 300 ms gesture, but it does not yet prove the changed |
…rt endpoint-hold in differential Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Re: the device-facing readiness requirement — I added runner-level trace evidence for the Maestro What changed:
A live iOS The |
|
Follow-up with a profile-sensitive iOS probe at I used the installed
So this is not merely submission evidence: the app-visible outcome distinguishes the iOS endpoint-hold path from timed-pan, and the PR head matches native Maestro on that probe. |
|
Thank you for running the profile-sensitive iOS probe — this is exactly the device-facing evidence needed. The pager-view result (native Maestro 2.5.1 advances to page 1, PR head advances to page 1, counterfactual public |
thymikee
added
the
ready-for-human
|
Code review is clean at |
|
Thank you for the thorough review and the profile-sensitive iOS probe. Glad the trace-invariant chain and the pager-view differential confirm the endpoint-hold delivery. No remaining blockers from my side. |
#1315 removed the timed forms of `swipe`, `gesture fling`, and `gesture swipe` and `gesture rotate`'s `velocity`, but shipped without the migration guide, the repository sweep, or the parse-time error that issue #1216's own checklist gates a removal on. The sweep finds what that left behind: both `06-swipe-gestures.ad` integration fixtures still carried the 5-argument swipe and would fail at replay, two tests still asserted the removed shapes, and two branches still read the retired positional. Argument arity for every public gesture syntax now lives in one table keyed off the canonical `GESTURE_KINDS`, so a new kind cannot skip it and a form removed from the CLI is removed from `.ad` in the same edit. Both callers read it: the CLI argv parse, and a new `.ad` preflight. A stale script now fails when it is parsed — before the replay executes any device action — naming the line and computing its rewrite, instead of running up to that step and failing as a repairable divergence. The preflight checks arity only: `${VAR}` tokens resolve after planning, and interpolation never splits a token, so the count is decidable while the values are not. Deleting the dead duration read in `readSwipeGeometry` would have left `replay export` emitting no duration, handing Maestro's 400ms default to a gesture the script runs at 100ms, so the export now states `duration: 100`. `.ad` positional gesture parsing is NOT removed. Its only remaining callers are the CLI argv parse and the `.ad` line parse, both the current public syntax rather than a bridge to an older one, so there is nothing to migrate off. ADR 0013 records that and drops the "compatibility" framing that made it read as debt. Both migrated fixtures verified on real devices with the repo's own CLI: iOS simulator 34.9s, Android emulator 45.9s.
… (#1393) * fix(gestures): fail pre-removal gesture forms at .ad parse time (#1216) #1315 removed the timed forms of `swipe`, `gesture fling`, and `gesture swipe` and `gesture rotate`'s `velocity`, but shipped without the migration guide, the repository sweep, or the parse-time error that issue #1216's own checklist gates a removal on. The sweep finds what that left behind: both `06-swipe-gestures.ad` integration fixtures still carried the 5-argument swipe and would fail at replay, two tests still asserted the removed shapes, and two branches still read the retired positional. Argument arity for every public gesture syntax now lives in one table keyed off the canonical `GESTURE_KINDS`, so a new kind cannot skip it and a form removed from the CLI is removed from `.ad` in the same edit. Both callers read it: the CLI argv parse, and a new `.ad` preflight. A stale script now fails when it is parsed — before the replay executes any device action — naming the line and computing its rewrite, instead of running up to that step and failing as a repairable divergence. The preflight checks arity only: `${VAR}` tokens resolve after planning, and interpolation never splits a token, so the count is decidable while the values are not. Deleting the dead duration read in `readSwipeGeometry` would have left `replay export` emitting no duration, handing Maestro's 400ms default to a gesture the script runs at 100ms, so the export now states `duration: 100`. `.ad` positional gesture parsing is NOT removed. Its only remaining callers are the CLI argv parse and the `.ad` line parse, both the current public syntax rather than a bridge to an older one, so there is nothing to migrate off. ADR 0013 records that and drops the "compatibility" framing that made it read as debt. Both migrated fixtures verified on real devices with the repo's own CLI: iOS simulator 34.9s, Android emulator 45.9s. * fix(gestures): reject removed swipe input at the Node/MCP boundary Review findings on d88c6ed. P1: `interactionDaemonWriters.swipe` hand-projects five fields, so a JavaScript caller's `durationMs` was dropped before the daemon's `readSwipeInput` could reject it and a default-duration fling ran instead — the exact silent reinterpretation the guide promises does not happen. `gesture` was already safe because its writer runs `readGestureInput` -> `readGesturePayload`, which rejects the removed keys; `swipe` was the one surface with no reader of its own. The rejection now lives in contracts and is shared by the client writer and the daemon handler, so there is one rule and one message. The SDK regression covers all four removed keys and asserts the transport is never reached; reverting the writer call fails it on `swipe durationMs`. P2: the preflight's retired-slot test required a numeric token, so `swipe 197 650 197 300 ${DURATION}` fell back to bare usage text. An unresolved `${VAR}` now counts as the retired slot and is carried into the pan rewrite, while a stray flag or word stays a plain usage error. P2: the removal shipped in 0.20.0, not 0.21 — removal commit 6d99914 is contained in tag v0.20.0. The guide said 0.21 because the CHANGELOG still files it under `Unreleased`; the tag is the truth (headings lag several releases repo-wide, so that is pre-existing and left alone). The `.ad` grep recipe now matches variable-backed durations too. * docs(gestures): make the migration sweep and MCP claim accurate Re-review findings on 328bad8 (both migration-guide accuracy). The saved-script sweep matched only the five-argument `swipe` form; it missed timed `gesture fling`, timed `gesture swipe`, and `gesture rotate ... velocity`, so the repository-cleanliness step was incomplete. Provide one grep per retired form (number-or-`${VAR}` token), each verified to flag the removed shape and skip the valid one. The MCP section claimed the structured rejection carries the CLI's concrete replacement command. It does not: `readGesturePayload` and `assertNoRemovedSwipeInput` return a message that names the removed key and the replacement command (e.g. `gesture fling does not accept durationMs; use gesture pan for timed movement`) but not the fully-substituted coordinate rewrite, because the structured request carries no positional string to rewrite. Describe what the structured path actually returns. * docs(gestures): make the .ad sweep parser-aligned Re-review finding on 4fd5d39: the documented sweep required literal single spaces and unquoted numeric tokens, but the `.ad` tokenizer separates on any whitespace (`/\s/`, so tabs too) and accepts double-quoted tokens, so tab-separated or quoted-duration stale lines the parser rejects were missed. Rewrite the patterns to follow the tokenizer: `[[:space:]]+` between tokens and a numeric slot that accepts a bare or double-quoted number (optionally signed) or `${VAR}`. Requiring a digit in the numeric slot keeps a trailing flag like `--count` from being read as the retired positional. Verified against a fixture of tab/space/quoted/quoted-var/negative encodings that the four greps flag exactly the lines `parseReplayScriptDetailed` rejects and none it accepts, and that the live repo sweeps clean. Also state plainly what a regex cannot promise: the parser is the authoritative gate — every retired form is rejected at parse time before any device action, so running the suite finds every stale line by construction and a missed grep can never reach execution. The grep stays a bulk pre-flight, and the "every affected line" claim is scoped to that.
1 participant
Summary
Removes the deprecated gesture duration and rotate-velocity inputs from all public surfaces (CLI, Node types, MCP schemas, daemon handlers, and
.adreplay) and updates the Maestro compatibility layer to route timed swipes togesture panwhile preserving theendpoint-holdexecution profile through an internal daemon seam.What changed
CLI grammar / help
swipenow accepts exactlyx1 y1 x2 y2(no trailingdurationMs).gesture fling <dir> x y [distance]no longer accepts a trailingdurationMs.gesture swipe <preset>no longer accepts a trailingdurationMs.gesture rotate <degrees> [x] [y]no longer accepts a trailingvelocity.Structured input / schemas
durationMsfromSwipeGesturePayload,FlingGesturePayload, andSwipeOptions/FlingOptions/SwipeGestureOptions.velocityfromRotateGesturePayloadandRotateGestureOptions.durationMsonswipe/flingorvelocityonrotate.durationMsremains valid ongesture panandgesture transform.Parsing / normalization
gesturePayloadFromPositionalsandswipePayloadFromPositionalsnow reject extra positionals with an actionableINVALID_ARGSmessage.readGesturePayloadrejectsdurationMsonfling/swipeandvelocityonrotate.readSwipeInputrejectsdurationMson coordinateswipe.GestureCompatibilityRule/GestureDeprecationplumbing and thepan+preset+durationMsplan variant.Maestro compatibility
projectSwipe()now maps Maestroswipeoperations togesture panwith the resolveddurationMsand a computeddelta, instead of emittingswipewithdurationMs.endpoint-holdexecution profile is threaded throughreq.internal.gestureExecutionProfileso iOS keeps the Maestro-compatible fast-swipe-then-hold behavior; the publicpansurface does not expose this profile.buildGesturePlanwithexecutionProfile: 'endpoint-hold'.Docs / tests
website/docs/docs/commands.md,CHANGELOG.md, and all affected unit/integration tests.gesture-normalization.test.tsandgesture-input.test.tsto assert rejection of removed forms and round-trip of canonical forms.Migration
swipe x1 y1 x2 y2 durationMsgesture pan x1 y1 (x2-x1) (y2-y1) durationMsgesture fling <dir> x y [distance] durationMsgesture pan <x> <y> <dx> <dy> durationMsgesture swipe <preset> durationMsgesture pan <x> <y> <dx> <dy> durationMsgesture rotate <degrees> [x] [y] velocitygesture rotate <degrees> [x] [y]swipe/gesture fling/gesture swipecontinue to be used for quick, duration-free throws.Closes #1218.
Related to #1216.
Link to Devin session: https://app.devin.ai/sessions/8bec5e78313446e180020b22e479c7ac
Requested by: @thymikee