feat(web): subs snapshots on a relay-opened carrier stream - #3568
feat(web): subs snapshots on a relay-opened carrier stream#3568paul-nechifor wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## paul/feat/w3-protocol-v5 #3568 +/- ##
============================================================
+ Coverage 77.42% 77.45% +0.03%
============================================================
Files 1266 1266
Lines 121551 121739 +188
Branches 10716 10733 +17
============================================================
+ Hits 94116 94299 +183
- Misses 24359 24364 +5
Partials 3076 3076
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 4 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Greptile SummaryThis change moves relay-to-robot subscription snapshots onto a reliable carrier stream and updates bridge-side control-frame handling. Two robot-disconnection paths remain: an oversized aggregate subscription snapshot is confirmed to fail the carrier, and rapid subscription changes can exhaust the carrier queue while its stream is flow-controlled. T-Rex validation blockedThe queue-overflow reproduction could not execute because the required Deno tool is not installed or available on Confidence Score: 1/5Not safe to merge until relay subscription updates are bounded against both carrier payload and pending-queue limits. Two independent robot availability failures require separate fixes: aggregate snapshots can exceed the control-frame budget, and changed snapshots can accumulate beyond the carrier queue limit during a stalled write. Files Needing Attention: web/relay/registry.ts, web/relay/carrier.ts, web/relay/session.ts
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const chs = this.#activeChs(robotId, entry.delivery); | ||
| if (!force && chs.join("\n") === entry.lastChs.join("\n")) return; | ||
| entry.lastChs = chs; | ||
| const msg: Msg = { t: "subs", chs, n: ++entry.n }; | ||
| const size = encodeDatagram(msg).byteLength; | ||
| if (size > DATAGRAM_BUDGET_BYTES) { | ||
| // Reachable since v5: a stream hello can declare a channel set whose | ||
| // full snapshot no longer fits one datagram (W4 moves snapshots to the | ||
| // reliable carrier). Loud because an oversized snapshot silently never | ||
| // reaches the robot. | ||
| console.error(`[relay] subs snapshot for ${robotId} is ${size} B (over datagram budget)`); | ||
| } | ||
| entry.peer.sendMsg(msg); | ||
| entry.peer.sendControl({ t: "subs", chs, n: ++entry.n }); |
There was a problem hiding this comment.
For a manifest-less robot, the relay accepts arbitrary distinct channel IDs up to the per-ID limit but does not limit the aggregate active-channel snapshot before forwarding it to sendControl. Once that encoded union exceeds the carrier's control-payload limit, RobotCarrier fails and the session layer closes the robot connection. Reject or otherwise bound a prospective subscription update when the deduplicated subs payload would exceed the carrier budget, rather than letting valid viewer requests terminate the robot session.
Artifacts
- Runs the real registry subscription path and RobotCarrier cap check with unique, individually valid 64-character IDs, ending with the aggregate-cap outcome.
- Executed 977 manifest-less subscriptions from `/home/user/repo/web`; the 65,487-byte snapshot remained below the cap and the carrier recorded no failure, establishing the boundary baseline.
- Executed 978 manifest-less subscriptions from `/home/user/repo/web`; the 65,554-byte snapshot exceeded the cap and RobotCarrier failed once, confirming the defect.
a35211d to
9e7202c
Compare
| if (msg.ch.length > MAX_MANIFEST_ID_LEN) { | ||
| reply({ | ||
| t: "error", | ||
| code: "unknown_channel", | ||
| message: `channel ids are at most ${MAX_MANIFEST_ID_LEN} chars`, | ||
| }); | ||
| break; | ||
| } |
There was a problem hiding this comment.
For a manifest-less robot, this guard limits each channel ID but not the accumulated active-channel set. A viewer can add enough individually valid distinct channels for the subs snapshot forwarded at line 526 to exceed MAX_CONTROL_PAYLOAD_BYTES; RobotCarrier.sendControl then fails the carrier and the session closes the robot connection. Bound or reject the prospective aggregate snapshot before sending it.
| const chs = this.#activeChs(robotId, entry.delivery); | ||
| if (!force && chs.join("\n") === entry.lastChs.join("\n")) return; | ||
| entry.lastChs = chs; | ||
| const msg: Msg = { t: "subs", chs, n: ++entry.n }; | ||
| const size = encodeDatagram(msg).byteLength; | ||
| if (size > DATAGRAM_BUDGET_BYTES) { | ||
| // Reachable since v5: a stream hello can declare a channel set whose | ||
| // full snapshot no longer fits one datagram (W4 moves snapshots to the | ||
| // reliable carrier). Loud because an oversized snapshot silently never | ||
| // reaches the robot. | ||
| console.error(`[relay] subs snapshot for ${robotId} is ${size} B (over datagram budget)`); | ||
| } | ||
| entry.peer.sendMsg(msg); | ||
| entry.peer.sendControl({ t: "subs", chs, n: ++entry.n }); |
There was a problem hiding this comment.
Every changed subscription set is immediately enqueued as a complete carrier snapshot. If the active stream write is flow-controlled, a viewer alternating valid sub and unsub requests can enqueue more than the carrier's 256-frame or 4 MiB limit. Its overflow path invokes carrier_failed, which closes the watched robot session. Coalesce pending snapshots, apply backpressure, or reject mutations before the queue limit is reached.
github-actions
Bot
added
the
ready-to-merge
- Subscription snapshots move from datagrams to @control frames on the carrier: one relay-opened reliable uni stream per robot session. - Snapshots are now ordered and size-unbounded. The periodic resend loop and the ~1200 B datagram budget are deleted. - Relay-opened uni streams are the direction proven to work on both legs. Handshake and teleop control stay on datagrams. - The carrier is a control dependency. A relay-side write failure or overflow fails the whole robot session (carrier_failed error + close). - The bridge treats corrupt carrier framing, a reset, or an early end the same way. It reconnects and the fresh registration re-baselines subs. - New web/relay/carrier.ts plus tests. The README gains a per-leg transport table.
9e7202c to
4c1d5a1
Compare
github-actions
Bot
removed
the
ready-to-merge
| const chs = this.#activeChs(robotId, entry.delivery); | ||
| if (!force && chs.join("\n") === entry.lastChs.join("\n")) return; | ||
| entry.lastChs = chs; | ||
| const msg: Msg = { t: "subs", chs, n: ++entry.n }; | ||
| const size = encodeDatagram(msg).byteLength; | ||
| if (size > DATAGRAM_BUDGET_BYTES) { | ||
| // Reachable since v5: a stream hello can declare a channel set whose | ||
| // full snapshot no longer fits one datagram (W4 moves snapshots to the | ||
| // reliable carrier). Loud because an oversized snapshot silently never | ||
| // reaches the robot. | ||
| console.error(`[relay] subs snapshot for ${robotId} is ${size} B (over datagram budget)`); | ||
| } | ||
| entry.peer.sendMsg(msg); | ||
| entry.peer.sendControl({ t: "subs", chs, n: ++entry.n }); |
There was a problem hiding this comment.
For a manifest-less robot, the per-channel length check does not bound the aggregate active-channel snapshot. The executed boundary reproduction accepted 978 distinct 64-character subscriptions, then encoded the subs control payload to 65,554 bytes—above the 65,536-byte control limit—which invokes the carrier failure path and closes the robot session. Reject a prospective subscription update when the production-encoded union would exceed the control budget, rather than forwarding an over-cap snapshot.
Artifacts
Drained carrier boundary reproduction harness
- Runs actual Registry and RobotCarrier code with one awaited drain per snapshot and calculates the adjacent serialized control-payload boundary, proving the aggregate behavior.
Control snapshot immediately below the payload limit
- The executed below-boundary run accepts 977 subscriptions at 65,487 B without carrier failure, showing the control path remains live below the cap.
Control snapshot immediately above the payload limit
- The executed above-boundary run accepts 978 subscriptions at 65,554 B then reports the production over-cap carrier failure, proving the robot session close condition.
| const chs = this.#activeChs(robotId, entry.delivery); | ||
| if (!force && chs.join("\n") === entry.lastChs.join("\n")) return; | ||
| entry.lastChs = chs; | ||
| const msg: Msg = { t: "subs", chs, n: ++entry.n }; | ||
| const size = encodeDatagram(msg).byteLength; | ||
| if (size > DATAGRAM_BUDGET_BYTES) { | ||
| // Reachable since v5: a stream hello can declare a channel set whose | ||
| // full snapshot no longer fits one datagram (W4 moves snapshots to the | ||
| // reliable carrier). Loud because an oversized snapshot silently never | ||
| // reaches the robot. | ||
| console.error(`[relay] subs snapshot for ${robotId} is ${size} B (over datagram budget)`); | ||
| } | ||
| entry.peer.sendMsg(msg); | ||
| entry.peer.sendControl({ t: "subs", chs, n: ++entry.n }); |
There was a problem hiding this comment.
Every changed subscription set is immediately enqueued as a complete carrier snapshot. If the active stream write is flow-controlled, a viewer alternating valid sub and unsub requests can enqueue more than the carrier's 256-frame or 4 MiB limit. Its overflow path invokes carrier_failed, which closes the watched robot session. Coalesce pending snapshots, apply backpressure, or reject mutations before the queue limit is reached.
Artifacts
Drained carrier boundary reproduction harness
- Runs actual Registry and RobotCarrier code with one awaited drain per snapshot and calculates the adjacent serialized control-payload boundary, proving the aggregate behavior.
Control snapshot immediately below the payload limit
- The executed below-boundary run accepts 977 subscriptions at 65,487 B without carrier failure, showing the control path remains live below the cap.
Control snapshot immediately above the payload limit
- The executed above-boundary run accepts 978 subscriptions at 65,554 B then reports the production over-cap carrier failure, proving the robot session close condition.
1 participant
No description provided.