Skip to content

refactor(tool-server): use @swmansion/argent-cloud-sdk for MoQ + input protocol - #743

Merged
jwajgelt merged 6 commits into
mainfrom
juliuszwajgelt/cld-27-use-argent-cloud-sdk
Aug 7, 2026
Merged

refactor(tool-server): use @swmansion/argent-cloud-sdk for MoQ + input protocol#743
jwajgelt merged 6 commits into
mainfrom
juliuszwajgelt/cld-27-use-argent-cloud-sdk

Conversation

@jwajgelt

@jwajgelt jwajgelt commented Aug 6, 2026

Copy link
Copy Markdown
Member

Summary

argent and the other client of the same device backend each hand-rolled the same MoQ client and DataChannelCommand protobuf encoder, with no shared source — argent carried a vendored copy of the .proto as an inline string that could silently drift from the server schema. Both now use @swmansion/argent-cloud-sdk, where the encoder is checked byte-for-byte against the canonical .proto in tests, so drift fails the build instead of shipping.

  • utils/moq-client.ts (195 → ~50 lines) is now a thin adapter over the SDK's MoqDeviceSession. It keeps the MoqClient interface exactly (sendControl, screenshotBuffer, close), so blueprints/simulator-server.ts and createMoqTransport are untouched. argent still resolves its endpoint through sim-remote moq-info (preserving daemon-based session sharing) and still publishes on its own "argent" broadcast path.
  • utils/datachannel-proto.ts (201 lines → re-exports) so its ~10 import sites keep working unchanged.
  • Dependencies: drops protobufjs and @moq/lite, both now unused. @moq/net is kept and still dedupes to a single copy.

No behavioural change intended.

Verified end-to-end against live infrastructure

Ran the migrated client against a real remote simulator (iPhone 17 Pro, iOS 26.5) on a live staging machine, driving this branch's moq-client.ts through the published SDK 0.1.2:

Check Result
MoQ session open (WebTransport, cert-pinned) PASS
Screenshot PASS — valid PNG 1206×2622
Sequential screenshots PASS
Concurrent screenshots PASS
Input round-trip (tap an app icon) PASS — tapping Settings opened Settings, confirmed visually in the returned screenshot
Screenshot after concurrent burst PASS
Session teardown PASS

Plus the existing suite: full workspace tsc --build, 3372 tests passing (1 skipped), and npm ls @moq/net confirming a single deduped copy.

A bug this surfaced, now fixed

Concurrent screenshot() calls hung under SDK 0.1.0. The server answers one screenshot per session at a time; 0.1.0 had dropped the serialisation the old hand-rolled client had, so a second in-flight request was never answered and its promise never settled — a regression against pre-SDK behaviour that only showed up under live traffic, with no unit test catching it.

Fixed in SDK 0.1.1 (queue requests, plus a 30s request timeout so a dropped response errors instead of hanging). This PR depends on ^0.1.2 — a republish of 0.1.1 through the release workflow carrying npm build provenance, whose shipped dist is byte-identical (verified by SHA256), so the live results above hold for it unchanged.

Notes for reviewers

  • The SDK subscribes to the screenshot track lazily, so a video-only consumer doesn't open a subscription it never reads. argent requests screenshots, so this path is exercised.
  • Screenshot responses are matched by the request id the server echoes, so a late or duplicate frame can't be handed to the wrong waiter.

…t protocol

The MoQ client and the DataChannelCommand encoder were duplicated by
hand between argent and the radon-cloud webui, with no shared source
and a vendored copy of the .proto that could silently drift. Both now
live in @swmansion/argent-cloud-sdk (radon-cloud/packages/argent-cloud-sdk),
where the encoder is checked byte-for-byte against the canonical schema
in tests.

- moq-client.ts becomes a thin adapter over the SDK's MoqDeviceSession,
  keeping the MoqClient interface (sendControl / screenshot -> Buffer /
  close) so the blueprint and createMoqTransport are untouched. argent
  keeps resolving the endpoint via `sim-remote moq-info` and still
  publishes on its own "argent" broadcast path.
- datachannel-proto.ts becomes re-exports from the SDK, so its ~10
  import sites keep working unchanged.
- Drops the now-unused protobufjs and @moq/lite dependencies; @moq/net
  still dedupes to a single copy in the packed bundle.
@jwajgelt jwajgelt self-assigned this Aug 6, 2026
@jwajgelt
jwajgelt requested review from filip131311 and latekvo August 6, 2026 09:42
This package is published publicly; the comments named a private repo.
0.1.1 serialises screenshot requests. Under 0.1.0 two concurrent
screenshot() calls hung forever, since the server answers one per
session at a time.

@latekvo latekvo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[Diplomat]: Reviewed end-to-end rather than by reading. What I ran against 189c4790, all green unless noted below: workspace tsc --build; npm test --workspaces (305 files / 3372 passed, 1 skipped in tool-server, matching the PR body, plus 87 / 282 / 31 elsewhere); typecheck:tests, typecheck:scripts, test:scripts, prettier and eslint on the changed files; the Lockfile CI gate (npm install --package-lock-only leaves package-lock.json unchanged); npm ls confirming a single deduped @moq/net@0.1.9 and no protobufjs / @moq/lite left in the tree; and packages/argent/scripts/bundle-tools.cjs, which inlines the SDK and @moq/net into dist/tool-server.cjs with no unresolved require, keeping only the existing @fails-components/webtransport dynamic import external.

Wire parity old-vs-new: 1627 vectors across all six encoders (every enum value including the proto3 zero cases, doubles at 0/-0/NaN/±Infinity/1e308/subnormals, float32 rounding, unicode and empty screenshot ids, secondX/secondY present/absent), old side driven by the merge-base's own PROTO_SOURCE through protobufjs@7.6.4 (the version the base lockfile pins). Identical bytes everywhere except encodeKey with a code outside int32 range or a negative non-integer/NaN — none of which any caller can produce, since codes only ever come from charToKeyPress (4–56), NAMED_KEYS (40–82), SHIFT_KEYCODE and the hardcoded CMD/V, so I am not raising it.

Behaviour: drove the published-shape esbuild bundle through a real @moq/net session (createMockTransportPair + Connection.connect/accept) against a fake simulator-server, with only globalThis.WebTransport faked — connectMoq, cert pinning, the handshake, MoqDeviceSession and ScreenshotChannel all real. Token appended to the URL, fingerprint decoded and pinned as one sha-256 hash, one transport attempt (no WebSocket race), screenshot round-trip byte-exact, touch/button/key/rotate decoding correctly server-side against the canonical .proto, and screenshot-after-close rejecting rather than hanging. Screenshot serialisation confirmed on the wire (5 concurrent calls, max 1 request in flight at the server, each caller getting its own image). Control-frame drops under an unpaced burst are identical before and after (6 of 7 lost in both, all 7 delivered in both once paced), so that is not a regression here. Two things below.

Comment thread packages/tool-server/src/utils/moq-client.ts
Comment thread packages/tool-server/src/utils/datachannel-proto.ts
0.1.2 is a republish of 0.1.1 through the release workflow, carrying npm
build provenance. The shipped dist is byte-identical; only packaging
metadata changed.
Removing the inline PROTO_SOURCE string left src/proto/datachannel.proto
behind. Nothing imports it, so there was no runtime effect, but it had
already fallen behind the server: no TouchPointer, no TouchStateCommand,
no touch_state arm on the oneof. It is the file a reader is most likely
to open when looking for this schema, so a stale copy is worse than none.

The SDK ships the canonical .proto in its published files, so point
readers there instead of keeping a copy that can drift again.
0.1.3 matches screenshot responses by id only. Previously a frame
without an id went to the oldest waiter, so the late answer to a
timed-out request could be returned to the next caller as a stale image
with nothing marking it as such.
@jwajgelt
jwajgelt merged commit 67a4481 into main Aug 7, 2026
5 checks passed
@jwajgelt
jwajgelt deleted the juliuszwajgelt/cld-27-use-argent-cloud-sdk branch August 7, 2026 10:51
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.

3 participants