Skip to content

Commit d6612fe

Browse files
committed
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.
1 parent e633ee4 commit d6612fe

6 files changed

Lines changed: 118 additions & 19 deletions

File tree

src/__tests__/client.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -673,6 +673,55 @@ test('interactions.rotateGesture rejects partial centers on the client side', as
673673
assert.equal(setup.calls.length, 0);
674674
});
675675

676+
test('removed gesture inputs are rejected client-side, never dropped from the projection', async () => {
677+
const setup = createTransport(async () => {
678+
throw new Error('transport should not run for invalid input');
679+
});
680+
const client = createAgentDeviceClient(setup.config, { transport: setup.transport });
681+
// A JavaScript caller (or a stale compiled build) can still reach these keys
682+
// past the removed compile-time fields. `swipe` has no structured reader of
683+
// its own, so before #1216 its writer silently dropped `durationMs` and ran a
684+
// default-duration fling instead of failing.
685+
const removed: Array<[string, () => Promise<unknown>, string]> = [
686+
[
687+
'swipe durationMs',
688+
() =>
689+
client.interactions.swipe({
690+
from: { x: 197, y: 650 },
691+
to: { x: 197, y: 300 },
692+
durationMs: 300,
693+
} as Parameters<typeof client.interactions.swipe>[0]),
694+
'swipe does not accept durationMs; use gesture pan for timed movement',
695+
],
696+
[
697+
'fling durationMs',
698+
() =>
699+
client.interactions.fling({ direction: 'down', x: 100, y: 200, durationMs: 300 } as never),
700+
'gesture fling does not accept durationMs; use gesture pan for timed movement',
701+
],
702+
[
703+
'gesture swipe durationMs',
704+
() => client.interactions.swipeGesture({ preset: 'left', durationMs: 300 } as never),
705+
'gesture swipe does not accept durationMs; use gesture pan for timed movement',
706+
],
707+
[
708+
'rotate velocity',
709+
() => client.interactions.rotateGesture({ degrees: 35, velocity: 800 } as never),
710+
'gesture rotate does not accept velocity; rotation pacing derives from degrees',
711+
],
712+
];
713+
714+
for (const [label, call, message] of removed) {
715+
await assert.rejects(
716+
call,
717+
(error: unknown) =>
718+
error instanceof AppError && error.code === 'INVALID_ARGS' && error.message === message,
719+
label,
720+
);
721+
}
722+
assert.equal(setup.calls.length, 0);
723+
});
724+
676725
test('interactions.pan projects one- and two-finger requests through typed gesture input', async () => {
677726
const setup = createTransport(async () => ({ ok: true, data: { message: 'Panned' } }));
678727
const client = createAgentDeviceClient(setup.config, { transport: setup.transport });

src/commands/interaction/interactions.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@ import {
1111
readInteractionTargetFromPositionals,
1212
} from '../../core/interaction-positionals.ts';
1313
import { AppError } from '../../kernel/errors.ts';
14-
import { swipePayloadFromPositionals } from '../../contracts/gesture-normalization.ts';
14+
import {
15+
assertNoRemovedSwipeInput,
16+
swipePayloadFromPositionals,
17+
} from '../../contracts/gesture-normalization.ts';
1518
import type { ScrollInputDirection } from './runtime/gestures.ts';
1619
import {
1720
commonInputFromFlags,
@@ -124,14 +127,16 @@ export const interactionDaemonWriters = {
124127
longpress: direct(PUBLIC_COMMANDS.longPress, (input) =>
125128
longPressPositionals(input as LongPressOptions),
126129
),
127-
swipe: (input) =>
128-
request(PUBLIC_COMMANDS.swipe, [], input, {
130+
swipe: (input) => {
131+
assertNoRemovedSwipeInput(input);
132+
return request(PUBLIC_COMMANDS.swipe, [], input, {
129133
from: input.from,
130134
to: input.to,
131135
count: input.count,
132136
pauseMs: input.pauseMs,
133137
pattern: input.pattern,
134-
}),
138+
});
139+
},
135140
focus: direct(PUBLIC_COMMANDS.focus, (input) => [String(input.x), String(input.y)]),
136141
type: direct(PUBLIC_COMMANDS.type, (input) => typePositionals(input as TypeTextOptions)),
137142
fill: direct(PUBLIC_COMMANDS.fill, (input) => fillPositionals(input as FillOptions)),

src/contracts/gesture-normalization.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,10 @@ test('a malformed line is a usage error rather than a migration claim', () => {
8787
code: 'INVALID_ARGS',
8888
message: 'swipe accepts 4 arguments: x1 y1 x2 y2.',
8989
});
90+
assert.throws(() => swipePayloadFromPositionals(['1', '2', '3', '4', 'hello']), {
91+
code: 'INVALID_ARGS',
92+
message: 'swipe accepts 4 arguments: x1 y1 x2 y2.',
93+
});
9094
});
9195

9296
test('the .ad preflight names the offending line and leaves live syntax alone', () => {
@@ -101,6 +105,26 @@ test('the .ad preflight names the offending line and leaves live syntax alone',
101105
assert.equal(describeReplayGestureArityError('click', ['120', '240'], 'line 2'), undefined);
102106
});
103107

108+
test('a variable-backed retired positional still gets the migration', () => {
109+
assert.equal(
110+
describeReplayGestureArityError('swipe', ['197', '650', '197', '300', '${DURATION}'], 'line 6'),
111+
'swipe accepts 4 arguments: x1 y1 x2 y2 (line 6). The trailing durationMs positional was removed: use "gesture pan 197 650 0 -350 ${DURATION}" for the same timed drag, or "swipe 197 650 197 300" for a default-duration swipe.',
112+
);
113+
assert.equal(
114+
describeReplayGestureArityError(
115+
'swipe',
116+
['${X1}', '${Y1}', '${X2}', '${Y2}', '${DURATION}'],
117+
'line 6',
118+
),
119+
'swipe accepts 4 arguments: x1 y1 x2 y2 (line 6). The trailing durationMs positional was removed: use "gesture pan x1 y1 dx dy durationMs" for the same timed drag, or "swipe ${X1} ${Y1} ${X2} ${Y2}" for a default-duration swipe.',
120+
);
121+
assert.match(
122+
describeReplayGestureArityError('gesture', ['rotate', '35', '195', '443', '${V}'], 'line 4') ??
123+
'',
124+
/trailing velocity positional was removed: use "gesture rotate 35 195 443"/,
125+
);
126+
});
127+
104128
test('the .ad preflight defers to dispatch for values it cannot know yet', () => {
105129
// `${VAR}` tokens resolve after planning, and interpolation never splits a
106130
// token, so only the argument count is decidable at parse time.

src/contracts/gesture-normalization.ts

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,7 @@ function describeGestureArityError(
114114
const syntax = PUBLIC_GESTURE_SYNTAX[key];
115115
if (args.length <= syntax.max) return undefined;
116116
const retired = syntax.retired;
117-
// A retired positional was always a number, so a single extra numeric
118-
// argument is the pre-removal form rather than a malformed line.
119-
if (!retired || args.length !== syntax.max + 1 || !isNumericArgument(args[syntax.max])) {
117+
if (!retired || args.length !== syntax.max + 1 || !isRetiredSlotArgument(args[syntax.max])) {
120118
return { usage: syntax.usage };
121119
}
122120
const canonical = `${key} ${args.slice(0, syntax.max).join(' ')}`;
@@ -126,8 +124,30 @@ function describeGestureArityError(
126124
};
127125
}
128126

129-
function isNumericArgument(value: string | undefined): boolean {
130-
return value !== undefined && value.trim().length > 0 && Number.isFinite(Number(value));
127+
/**
128+
* Whether one extra argument occupies the retired slot. A retired positional was
129+
* always a number, and a `.ad` script may hold it as an unresolved `${VAR}`, so
130+
* both report the migration while a stray flag or word stays a usage error.
131+
*/
132+
function isRetiredSlotArgument(value: string | undefined): boolean {
133+
if (value === undefined || value.trim().length === 0) return false;
134+
return value.startsWith('${') || Number.isFinite(Number(value));
135+
}
136+
137+
/**
138+
* `swipe` is the one public gesture surface whose structured input has no reader
139+
* of its own — the daemon writer projects known fields — so a removed key would
140+
* be dropped before the daemon could reject it and a default-duration fling
141+
* would run instead. Rejecting here covers the Node and MCP boundaries at the
142+
* same point `readGesturePayload` rejects the gesture kinds' removed keys.
143+
*/
144+
export function assertNoRemovedSwipeInput(input: unknown): void {
145+
if (!input || typeof input !== 'object' || Array.isArray(input)) return;
146+
if ((input as Record<string, unknown>).durationMs === undefined) return;
147+
throw new AppError(
148+
'INVALID_ARGS',
149+
'swipe does not accept durationMs; use gesture pan for timed movement',
150+
);
131151
}
132152

133153
function formatGestureArityError(error: GestureArityError, source?: string): string {

src/daemon/handlers/interaction-gesture.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { readGesturePayload, type GesturePayload } from '../../contracts/gesture-input.ts';
22
import {
3+
assertNoRemovedSwipeInput,
34
gesturePayloadToPositionals,
45
normalizePublicGesture,
56
normalizePublicSwipeMotion,
@@ -189,13 +190,8 @@ function readSwipeInput(input: unknown): SwipePayload {
189190
if (!input || typeof input !== 'object' || Array.isArray(input)) {
190191
throw new AppError('INVALID_ARGS', 'swipe requires structured object input');
191192
}
193+
assertNoRemovedSwipeInput(input);
192194
const record = input as Record<string, unknown>;
193-
if (record.durationMs !== undefined) {
194-
throw new AppError(
195-
'INVALID_ARGS',
196-
'swipe does not accept durationMs; use gesture pan for timed movement',
197-
);
198-
}
199195
const pattern = record.pattern;
200196
if (pattern !== undefined && pattern !== 'one-way' && pattern !== 'ping-pong') {
201197
throw new AppError('INVALID_ARGS', 'swipe pattern must be one-way or ping-pong');

website/docs/docs/migrating-gestures.md

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ title: Migrating Gestures
44

55
# Migrating Gestures
66

7-
`agent-device` 0.21 removed the timed forms of `swipe`, `gesture fling`, and `gesture swipe`, and
7+
`agent-device` 0.20.0 removed the timed forms of `swipe`, `gesture fling`, and `gesture swipe`, and
88
the `velocity` argument of `gesture rotate`. Nothing is silently reinterpreted: every removed form
99
now fails with an `INVALID_ARGS` error that names its replacement.
1010

@@ -61,7 +61,9 @@ for a default-duration swipe.
6161

6262
`interactions.swipe`, `interactions.fling`, and `interactions.swipeGesture` no longer accept
6363
`durationMs`; `interactions.rotateGesture` no longer accepts `velocity`. Passing them is a type
64-
error at compile time and an `INVALID_ARGS` rejection at runtime.
64+
error at compile time and an `INVALID_ARGS` rejection at runtime — the rejection happens
65+
client-side, before the request reaches the daemon, so a plain JavaScript caller or a stale
66+
compiled build gets the error rather than a silently retimed gesture.
6567

6668
```ts
6769
// before
@@ -114,10 +116,13 @@ swipe 206 650 206 300 300 --count 2 --pause-ms 200 --pattern one-way
114116
swipe 206 650 206 300 --count 2 --pause-ms 200 --pattern one-way
115117
```
116118

119+
A duration held in a variable (`swipe 197 650 197 300 ${DURATION}`) is reported the same way — the
120+
preflight counts arguments, so it does not need the value.
121+
117122
To find every affected line across a suite:
118123

119124
```bash
120-
grep -rnE '\bswipe +[-0-9.]+ +[-0-9.]+ +[-0-9.]+ +[-0-9.]+ +[-0-9.]+' --include='*.ad' .
125+
grep -rnE '\bswipe( +([-0-9.]+|\$\{[^}]*\})){5}' --include='*.ad' .
121126
```
122127

123128
Re-recording also produces a migrated script: the recorder writes the canonical form, so a fresh
@@ -154,7 +159,7 @@ to clear:
154159
that can appear in a saved recording, the rejection must fire at parse time and name the line, so
155160
a stale script never half-executes.
156161

157-
The 0.21 removal completed all five steps.
162+
The 0.20.0 removal completed all five steps.
158163

159164
## Positional `.ad` syntax
160165

0 commit comments

Comments
 (0)