Files
software-mansion__argent/packages/tool-server/test/gesture-rotate-radius.test.ts
Ignacy Łątka a2ed83e041 fix(tool-server): await the simulator-server command ack so a lost tap fails (#985)
## The defect

`sendCommand` returns `void`. It writes the JSON command onto the
WebSocket — or queues it behind `once("open")` — and returns without
ever reading the reply:

```ts
export function sendCommand(api, cmd): void {
  const ws = getOrCreateWs(api);
  const payload = JSON.stringify({ id: String(++cmdId), ...cmd });
  if (ws.readyState === WebSocket.OPEN) ws.send(payload);
  else ws.once("open", () => ws.send(payload));   // queued, never awaited
}
```

simulator-server *does* acknowledge every command; argent just never
looked. Probing the shipped binary's `/ws` endpoint:

| sent | reply |
|---|---|
| `{"id":"1","cmd":"touch","type":"Down",…}` |
`{"id":"1","status":"ok"}` |
| `{"id":"7","cmd":"button","button":"notARealButton"}` |
`{"status":"error","message":"parse error: unknown variant
`notARealButton`…"}` |

Because the return type was `void` there was no ack to check and no
error to surface, so `gesture-tap` ended in an unconditional `return {
tapped: true, timestampMs }`.

**Repro:** shut a simulator down underneath an open session. The server
sends no reply at all and `gesture-tap` still reports `{ tapped: true
}`. Every transport failure — that one, a dead socket, a wedged device —
looks identical to success from the agent's side.

This is argent's own bug, and it is what turns an Apple regression into
a silent one: awaiting the reply would have made #932 an actionable
failure on day one instead of a silent no-op.

## The fix

`sendCommand` now returns a promise that resolves on `{"status":"ok"}`
and rejects when:

| condition | `error_code` |
|---|---|
| server answers `status: "error"` | `SIMULATOR_COMMAND_REJECTED` |
| socket fails or closes with the command in flight |
`SIMULATOR_COMMAND_TRANSPORT_FAILED` |
| reachable server never answers (5s) | `SIMULATOR_COMMAND_ACK_TIMEOUT`
|

Every WebSocket call site awaits it: `gesture-tap`, `gesture-swipe`,
`gesture-custom`, `gesture-pinch`, `gesture-rotate`, `button`, `rotate`.

Two details of the wire protocol shaped the implementation:

- **A success reply echoes the command id; an error reply carries no id
at all.** So a rejection can only be matched positionally, against the
oldest outstanding command. Since every send is awaited, that is
normally the only one in flight.
- **The abort paths in `gesture-swipe` / `gesture-rotate` lift the
finger before unwinding, and that lift is now fallible.** It is
therefore best-effort: a refused lift rides along as the `AbortError`'s
`cause` rather than replacing it — matching the existing pattern in
`gesture-drag`, because `format-error.ts` and `cdp-session.ts` key on
`err.name === "AbortError"`.

## Verified on a real simulator

Before, after shutting the device down mid-session: `{"tapped": true}`.
After:

```
result: {"tapped":true,"timestampMs":…}          ← healthy tap, unchanged

--- simulator shut down underneath the open session ---
tap FAILED as it should:
   code: SIMULATOR_COMMAND_TRANSPORT_FAILED
   msg : simulator-server did not accept the 'touch' command: connect ECONNREFUSED
         127.0.0.1:62818. The command was NOT delivered to the device. …
```

The failure signal is fully populated for telemetry (`error_kind:
"network"`, `network_failure: "connection_reset"`, `failure_command:
"simulator_server"`).

## Cost

An ack is a localhost round-trip behind an already-open socket: **0.06ms
p50 / 0.17ms max**, measured over 200 sends against a booted iOS sim.
Awaiting one per event adds a flat **~30ms to a swipe of any duration**
— against the 36–142ms of `setTimeout` skew the 16ms frame loop already
carries:

| `durationMs` | wall before | wall after | added |
|---|---|---|---|
| 200 | 236ms | 266ms | +30ms |
| 500 | 559ms | 591ms | +31ms |
| 1000 | 1142ms | 1178ms | +35ms |

Flagging it explicitly since the swipe cadence is deliberately tuned. If
reviewers would rather keep the cadence byte-identical, the alternative
is to pipeline within a gesture and await all acks at the end; that
costs more code and defers failure to the end of the gesture, so I kept
the simpler per-send await.

## Deliberately out of scope

- **The MoQ (`ios-remote`) transport** is unchanged — it has no
equivalent ack.
- **The stdin `pressKey` channel**, used by `keyboard` and paste's ⌘V,
is untouched: simulator-server never replies on stdin, so it has the
same blind spot with no local remedy. The WebSocket protocol *does*
expose an acked `key` command
(`{"cmd":"key","direction":"Down","code":4}` → `{"status":"ok"}`), so
moving those onto it would close the gap — a separate, riskier change to
the text-entry path that deserves its own PR.

## Tests

`packages/tool-server/test/simulator-command-ack.test.ts` — 8 tests
against a fake `/ws` server that mirrors the real wire format, **all 8
fail on `main`**: ack resolves, `status: error` rejects, socket death
rejects in flight, silent server times out, no phantom success,
`gesture-tap` throws instead of claiming a rejected tap, and the aborted
swipe still reports `AbortError`.

Full suite: 369 files / 4840 tests pass, `npm run lint` clean, `tsc
--build` clean.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_011bEvNDV7o3CpKdFmzjVBYF

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Simulator commands now confirm successful delivery before completing.
* Added clear errors for rejected commands, connection failures, and
acknowledgement timeouts.
* **Bug Fixes**
* Button, rotation, tap, swipe, pinch, and custom gestures now report
failures instead of silently succeeding.
* In-progress gestures handle connection interruptions and cleanup
failures more reliably.
* **Tests**
* Added coverage for command acknowledgements, rejections, timeouts,
transport failures, and gesture error handling.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: latekvo <f.kaminski2000@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-31 18:31:53 +02:00

176 lines
6.2 KiB
TypeScript

import { describe, expect, it, vi, beforeEach } from "vitest";
// Keep the real module (blueprints import from it too) but neutralise the
// fire-and-forget WebSocket send so no real socket is opened during the test.
vi.mock("../src/utils/simulator-client", async (importOriginal) => ({
...(await importOriginal<typeof import("../src/utils/simulator-client")>()),
sendCommand: vi.fn(),
}));
import { zodObjectToJsonSchema } from "@argent/registry";
import { gestureRotateTool } from "../src/tools/gesture-rotate";
import { sendCommand } from "../src/utils/simulator-client";
const udid = "AAAAAAAA-AAAA-AAAA-AAAA-AAAAAAAAAAAA";
const services = { simulatorServer: {} } as never;
// The physical screen the normalized coordinates land on in these assertions.
const SCREEN_W = 1080;
const SCREEN_H = 2400;
interface TouchCmd {
cmd: string;
type: "Down" | "Move" | "Up";
x: number;
y: number;
second_x: number | null;
second_y: number | null;
}
function touches(): TouchCmd[] {
return vi.mocked(sendCommand).mock.calls.map(([, cmd]) => cmd as unknown as TouchCmd);
}
beforeEach(() => {
// Reset (not just clear) so the abort test's mockImplementation can't leak.
vi.mocked(sendCommand).mockReset();
});
describe("gesture-rotate radiusX/radiusY", () => {
it("orbits a physical circle with constant finger separation when both are set", async () => {
// One physical radius, two normalizations: 0.4·1080 = 0.18·2400 = 432 px.
await gestureRotateTool.execute(services, {
udid,
centerX: 0.5,
centerY: 0.5,
radiusX: 0.4,
radiusY: 0.18,
startAngle: 0,
endAngle: 90,
durationMs: 48, // 3 steps → 4 frames, keeps the test fast
});
const frames = touches();
expect(frames).toHaveLength(4);
expect(frames[0]!.type).toBe("Down");
expect(frames[3]!.type).toBe("Up");
for (const f of frames) {
const px1 = f.x * SCREEN_W;
const py1 = f.y * SCREEN_H;
const px2 = f.second_x! * SCREEN_W;
const py2 = f.second_y! * SCREEN_H;
// Each finger stays 432 physical px from the center; separation 864.
expect(Math.hypot(px1 - 0.5 * SCREEN_W, py1 - 0.5 * SCREEN_H)).toBeCloseTo(432, 9);
expect(Math.hypot(px1 - px2, py1 - py2)).toBeCloseTo(864, 9);
}
});
it("keeps the legacy radius-only behavior byte-identical", async () => {
await gestureRotateTool.execute(services, {
udid,
centerX: 0.4,
centerY: 0.6,
radius: 0.15,
startAngle: 0,
endAngle: 90,
durationMs: 48,
});
const frames = touches();
expect(frames).toHaveLength(4);
frames.forEach((f, i) => {
// The pre-radiusX formula, replicated operation-for-operation so the
// expected values are bit-identical, not merely close.
const t = i / 3;
const angleDeg = 0 + (90 - 0) * t;
const angleRad = (angleDeg * Math.PI) / 180;
expect(f.x).toBe(0.4 + 0.15 * Math.cos(angleRad));
expect(f.y).toBe(0.6 + 0.15 * Math.sin(angleRad));
expect(f.second_x!).toBe(0.4 - 0.15 * Math.cos(angleRad));
expect(f.second_y!).toBe(0.6 - 0.15 * Math.sin(angleRad));
});
});
});
describe("gesture-rotate inputSchema", () => {
it("keeps the radius trio optional at the top level", () => {
const schema = zodObjectToJsonSchema(gestureRotateTool.zodSchema!);
expect(schema.type).toBe("object");
const required = schema.required as string[];
expect(required).toEqual(
expect.arrayContaining(["udid", "centerX", "centerY", "startAngle", "endAngle"])
);
expect(required).not.toContain("radius");
expect(required).not.toContain("radiusX");
expect(required).not.toContain("radiusY");
expect(schema).not.toHaveProperty("$schema");
// The cross-field rule cannot live in the schema a client may receive (see
// tool-input-schema-contract.test.ts), so the description has to carry it.
expect(gestureRotateTool.description).toMatch(/one half of the pair alone/i);
});
it("the zod refinements accept exactly the documented radius shapes", () => {
const base = { udid, centerX: 0.5, centerY: 0.5, startAngle: 0, endAngle: 90 };
const shapes: Array<[radii: Record<string, number>, valid: boolean]> = [
[{ radius: 0.15 }, true],
[{ radiusX: 0.4, radiusY: 0.18 }, true],
[{ radius: 0.15, radiusX: 0.4, radiusY: 0.18 }, true],
[{}, false],
[{ radius: 0.15, radiusX: 0.4 }, false],
[{ radius: 0.15, radiusY: 0.18 }, false],
[{ radiusX: 0.4 }, false],
[{ radiusY: 0.18 }, false],
];
for (const [radii, valid] of shapes) {
const params = { ...base, ...radii };
expect(
gestureRotateTool.zodSchema!.safeParse(params).success,
`zod schema on ${JSON.stringify(radii)}`
).toBe(valid);
}
});
});
// The ctx-less path (execute called with no third argument, as in direct unit
// invocations) is covered by every test in the describe above.
describe("gesture-rotate abort", () => {
it("lifts the fingers where they are and rejects when aborted mid-gesture", async () => {
const controller = new AbortController();
// Abort synchronously from inside the send mock after the 3rd dispatched
// event — deterministic, no real-time races.
let sent = 0;
vi.mocked(sendCommand).mockImplementation(() => {
if (++sent === 3) controller.abort();
return Promise.resolve();
});
await expect(
gestureRotateTool.execute(
services,
{
udid,
centerX: 0.5,
centerY: 0.5,
radius: 0.15,
startAngle: 0,
endAngle: 90,
durationMs: 480, // 30 steps → 31 frames if run to completion
},
{ signal: controller.signal } as never
)
).rejects.toThrow(/gesture-rotate aborted/);
const frames = touches();
// Down + 2 Moves before the abort lands, then only the terminal Up.
expect(frames.map((f) => f.type)).toEqual(["Down", "Move", "Move", "Up"]);
// The lift happens at the last dispatched finger positions, not the arc end.
const last = frames[3]!;
const prev = frames[2]!;
expect(last.x).toBe(prev.x);
expect(last.y).toBe(prev.y);
expect(last.second_x).toBe(prev.second_x);
expect(last.second_y).toBe(prev.second_y);
});
});