feat(web): protocol v5, robot hello on an @control stream - #3567
feat(web): protocol v5, robot hello on an @control stream#3567paul-nechifor wants to merge 1 commit into
Conversation
- Protocol v5. The robot's hello moves from lossy datagrams to an @control data frame on a fresh one-shot bidi stream, resent until welcome arrives. - The hello payload keeps the datagram encoding but may now be 64 KiB, which frees the manifest from the ~1100 B datagram budget. - Channel ids beginning with @ are reserved for protocol control. Manifests using them and datagram hellos are rejected with explicit reject reasons. - Shared fixtures regenerated for v5. Relay session/forward/registry and the Python bridge client change together, pinned by the golden wire vectors. - server_test.ts: the v5 handshake updates are applied on top of the deflaked backpressure test from main (#3555). This branch adds no flake fix of its own.
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
Greptile SummaryThis change introduces protocol v5, carries robot hello messages on the reserved control stream, and expands supported manifest sizes. Two reliability issues were reproduced: valid large manifests can generate subscription snapshots that exceed the robot datagram limit, and delayed transport acknowledgements can cause hello retries to reset streams before the relay reads them. These failures can leave robots without current subscription state or prevent them from completing the relay handshake. Confidence Score: 3/5Not safe to merge until the subscription-state delivery and hello retry behavior are made reliable under supported large-manifest and delayed-acknowledgement conditions. There are two independent P1 findings and neither is security-related, which yields a score of 3. Both were reproduced with focused runtime executions using the affected production paths. Files Needing Attention: web/relay/registry.ts needs a delivery mechanism or size bound for complete subscription snapshots, and dimos/web/relay_bridge/wt_client.py needs retry cleanup that does not reset hello streams solely because they remain unacknowledged.
 T-Rex LogsWhat T-Rex did
Reviews (1): Last reviewed commit: "feat(web): protocol v5, robot hello on a..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const msg: Msg = { t: "subs", chs, n: ++entry.n }; | ||
| const size = encodeDatagram(msg).byteLength; | ||
| if (size > DATAGRAM_BUDGET_BYTES) { | ||
| // Unreachable while subs are manifest-validated (a manifest that fit | ||
| // its hello datagram implies a fitting snapshot). Loud if it ever | ||
| // happens: an oversized snapshot silently never reaches the robot. | ||
| // 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); |
There was a problem hiding this comment.
A valid large manifest can produce a complete subs snapshot larger than the 1200-byte datagram budget. This branch only logs the overflow and still sends the single oversized datagram, so the robot never receives the authoritative subscription state; periodic snapshot retries remain oversized as the active set grows. Send this state on a reliable control stream, fragment it, or enforce a channel-count limit that keeps its encoded snapshot within the datagram budget.
Artifacts
Focused large-manifest reproduction source
- Executes the registry subscription path with a valid 32-channel manifest, a 1200-byte datagram contract, and a follow-up teleop control message; takeaway: the source directly exercises the claimed oversized-snapshot path.
PR oversized-snapshot runtime output
- Captured PR execution shows protocol v5 accepting the large manifest and sending a 1211-byte subscription snapshot over the 1200-byte budget while follow-up teleop remains accepted; takeaway: valid v5 manifests can produce an undeliverable subscription snapshot.
| def retire_hello_stream() -> None: | ||
| # In-flight check and reset in the same event-loop turn (the | ||
| # aioquic-safe reset rule, web/README.md bug 9); a delivered | ||
| # stream is left alone so a reset cannot destroy a hello the | ||
| # relay has yet to read. | ||
| if hello_stream is not None and self._session.stream_in_flight(hello_stream): | ||
| self._session.reset_if_in_flight(hello_stream) | ||
|
|
||
| try: | ||
| while True: | ||
| if control_payload is None: | ||
| self._session.send_msg(msg) | ||
| else: | ||
| retire_hello_stream() | ||
| hello_stream = self.send_frame(CONTROL_CHANNEL, control_payload) | ||
| with contextlib.suppress(asyncio.TimeoutError): | ||
| await asyncio.wait_for(self._session.welcomed.wait(), 0.2) |
There was a problem hiding this comment.
stream_in_flight() only means the sender has not received a transport ACK; it does not mean the relay has not yet consumed the stream. After each 200 ms wait, this retry path resets that still-deliverable hello stream before resending. When ACK and relay consumption are delayed beyond the retry interval, every hello is reset before the relay reads it and the robot handshake times out. Keep prior hello streams live until a terminal handshake result, connection close, or a protocol-level acknowledgement that proves they can be retired.
Artifacts
Focused delayed-ack robot hello reproduction source
- The executed Python harness drives RelayClient.hello against a transport that delays ACK and relay consumption to 350 ms, showing whether reset destroys the still-deliverable stream.
Delayed-ack handshake without destructive stream resets
- The identical delayed transport completed welcome at 351 ms and recorded relay consumption of stream 101 when resets did not cancel delivery, proving the stream remained deliverable.
Current retry behavior with destructive stream resets
- The current RelayClient.hello behavior reset streams 101 through 104, recorded no relay consumption, and timed out after 803 ms, confirming the claimed failure.
1 participant
No description provided.