fix(cli): gesture accepts an array of steps so drags/swipes work in one call#127
Open
barkleesanders wants to merge 1 commit into
Open
fix(cli): gesture accepts an array of steps so drags/swipes work in one call#127barkleesanders wants to merge 1 commit into
barkleesanders wants to merge 1 commit into
Conversation
…ne call
`serve-sim gesture` parsed exactly one {type,x,y} object, sent a single
0x03 frame, and closed the socket 50ms later. Since a complete touch must
ride one WebSocket, the CLI could not express a drag: splitting
begin/move/end across invocations registers as a long-press (in a webview
it triggers text selection), and passing a JSON array parsed without error
but silently did nothing. The bundled agent skill even recommended the
multi-call form and noted it "may produce a long-press on some hosts".
gesture now accepts either the existing single-object form (unchanged) or
a JSON array of steps replayed in order on one socket, waiting per-step
`delayMs` (default 16ms, ~one 60fps frame) between sends. A trailing
settle (e.g. delayMs: 80 on the last move) turns a flick into a
positional drag. Multi-touch (x1/y1/x2/y2) and `edge` pass through
unchanged; invalid input now fails loudly naming the failing step instead
of silently no-opping.
Verified live on an iPhone 17 Pro simulator (iOS 26.5) against a WKWebView
app: the array form scrolls; the old multi-call recipe long-pressed and the
array form previously did nothing. Skill docs updated to match.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BEtZcizxUo7x9da5MLpHqC
📝 WalkthroughWalkthrough
ChangesGesture sequence replay
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/serve-sim/src/gesture-steps.ts`:
- Around line 47-53: Update the coordinate validation around `single` and
`multi` to reject steps where both coordinate forms are valid. Add a guard for
`single && multi` that throws the same invalid-coordinate error before
constructing or sending `stepPayload`, ensuring mixed single-touch and
multi-touch fields cannot reach the server.
In `@packages/serve-sim/src/index.ts`:
- Around line 713-742: Import WebSocket from the package dependency "ws" at the
top of the module, and use that imported symbol in the existing WebSocket
construction within the replay function. Remove reliance on the Node global
while preserving the current connection behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 69d8a95b-205e-426f-ac63-5cb39c96ec8c
📒 Files selected for processing (5)
packages/serve-sim/src/__tests__/gesture-steps.test.tspackages/serve-sim/src/gesture-steps.tspackages/serve-sim/src/index.tsskills/serve-sim/SKILL.mdskills/serve-sim/references/gestures.md
Comment on lines
+47
to
+53
| const single = isNormalized(x) && isNormalized(y); | ||
| const multi = isNormalized(x1) && isNormalized(y1) && isNormalized(x2) && isNormalized(y2); | ||
| if (!single && !multi) { | ||
| throw new Error( | ||
| `Invalid ${label}: needs normalized 0..1 coords — either {"x","y"} or {"x1","y1","x2","y2"} — got ${JSON.stringify(step)}`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject steps with both single-touch and multi-touch coordinates.
When both {x, y} and {x1, y1, x2, y2} are present and valid, validation passes and stepPayload sends all fields to the server, creating an ambiguous wire payload. Add a guard to reject mixed coordinates.
🛡️ Proposed fix
const single = isNormalized(x) && isNormalized(y);
const multi = isNormalized(x1) && isNormalized(y1) && isNormalized(x2) && isNormalized(y2);
+ if (single && multi) {
+ throw new Error(
+ `Invalid ${label}: provide either {"x","y"} or {"x1","y1","x2","y2"}, not both — got ${JSON.stringify(step)}`,
+ );
+ }
if (!single && !multi) {
throw new Error(
`Invalid ${label}: needs normalized 0..1 coords — either {"x","y"} or {"x1","y1","x2","y2"} — got ${JSON.stringify(step)}`,
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const single = isNormalized(x) && isNormalized(y); | |
| const multi = isNormalized(x1) && isNormalized(y1) && isNormalized(x2) && isNormalized(y2); | |
| if (!single && !multi) { | |
| throw new Error( | |
| `Invalid ${label}: needs normalized 0..1 coords — either {"x","y"} or {"x1","y1","x2","y2"} — got ${JSON.stringify(step)}`, | |
| ); | |
| } | |
| const single = isNormalized(x) && isNormalized(y); | |
| const multi = isNormalized(x1) && isNormalized(y1) && isNormalized(x2) && isNormalized(y2); | |
| if (single && multi) { | |
| throw new Error( | |
| `Invalid ${label}: provide either {"x","y"} or {"x1","y1","x2","y2"}, not both — got ${JSON.stringify(step)}`, | |
| ); | |
| } | |
| if (!single && !multi) { | |
| throw new Error( | |
| `Invalid ${label}: needs normalized 0..1 coords — either {"x","y"} or {"x1","y1","x2","y2"} — got ${JSON.stringify(step)}`, | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/serve-sim/src/gesture-steps.ts` around lines 47 - 53, Update the
coordinate validation around `single` and `multi` to reject steps where both
coordinate forms are valid. Add a guard for `single && multi` that throws the
same invalid-coordinate error before constructing or sending `stepPayload`,
ensuring mixed single-touch and multi-touch fields cannot reach the server.
Comment on lines
+713
to
+742
| let steps: GestureStep[]; | ||
| try { | ||
| touch = JSON.parse(jsonStr); | ||
| } catch { | ||
| console.error("Invalid JSON:", jsonStr); | ||
| steps = parseGestureSteps(jsonStr); | ||
| } catch (error) { | ||
| console.error(error instanceof Error ? error.message : String(error)); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| return new Promise<void>((resolve, reject) => { | ||
| const ws = new WebSocket(state.wsUrl); | ||
| ws.binaryType = "arraybuffer"; | ||
|
|
||
| ws.onopen = () => { | ||
| const send = (touch: ReturnType<typeof stepPayload>) => { | ||
| const json = new TextEncoder().encode(JSON.stringify(touch)); | ||
| const msg = new Uint8Array(1 + json.length); | ||
| msg[0] = 0x03; | ||
| msg.set(json, 1); | ||
| ws.send(msg); | ||
| }; | ||
| const sleep = (ms: number) => new Promise<void>((r) => setTimeout(r, ms)); | ||
|
|
||
| // All steps replay on THIS socket: a begin→move×N→end sequence split | ||
| // across CLI invocations (one socket each) registers as a long-press. | ||
| ws.onopen = async () => { | ||
| for (const [i, step] of steps.entries()) { | ||
| send(stepPayload(step)); | ||
| if (i < steps.length - 1) { | ||
| await sleep(step.delayMs ?? DEFAULT_STEP_DELAY_MS); | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether WebSocket is imported from the `ws` package or used as global in index.ts.
# Search for ws-related imports at the top of the file
rg -n 'import.*WebSocket.*from.*ws' packages/serve-sim/src/index.ts
# Also check for any require('ws') patterns
rg -n "require\(['\"]ws['\"]\)" packages/serve-sim/src/index.ts
# Show all import lines to see if WebSocket is imported
sed -n '1,15p' packages/serve-sim/src/index.tsRepository: EvanBacon/serve-sim
Length of output: 1141
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find every WebSocket reference in the file.
rg -n '\bWebSocket\b' packages/serve-sim/src/index.ts
# Show the surrounding code where WebSocket is used.
sed -n '700,760p' packages/serve-sim/src/index.ts
# Check whether ws is declared in package.json dependencies for this package.
sed -n '1,220p' packages/serve-sim/package.jsonRepository: EvanBacon/serve-sim
Length of output: 5436
Import WebSocket from ws here
new WebSocket(state.wsUrl) relies on the Node global. Import it from ws instead so this path stays consistent with the package dependency and portable across supported runtimes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/serve-sim/src/index.ts` around lines 713 - 742, Import WebSocket
from the package dependency "ws" at the top of the module, and use that imported
symbol in the existing WebSocket construction within the replay function. Remove
reliance on the Node global while preserving the current connection behavior.
Source: Coding guidelines
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
serve-sim gestureparses exactly one{type,x,y}object, sends a single0x03frame, and closes the socket ~50ms later (gesture()inpackages/serve-sim/src/index.ts). Since a complete touch must ride one WebSocket, the CLI cannot express a drag/swipe/scroll:begin/move/endacross invocations registers as a long-press — in a WKWebView it triggers text selection instead of scrolling.Fix
gesturenow accepts either the existing single-object form (unchanged, back-compat) or a JSON array of steps replayed in order on one socket, waiting per-stepdelayMs(default 16ms ≈ one 60fps frame) between sends:The
delayMs: 80settle beforeendreads as a positional drag; without it you get a momentum flick. Multi-touch (x1/y1/x2/y2) andedgepass through unchanged. Invalid input now fails loudly, naming the failing step (step 2/3: "type" must be one of begin|move|end), instead of silently no-opping.gesture-steps.tsmodule (parse + validate, mirrors thetext-to-keys.tspattern) withbun:testcoverage (12 tests).Verification
bun run typecheckclean (incl.noUncheckedIndexedAccess),bun run lint0/0, new tests 12/12.🤖 Generated with Claude Code
https://claude.ai/code/session_01BEtZcizxUo7x9da5MLpHqC
Summary by CodeRabbit
New Features
Documentation