Skip to content

Relay follow-ups from #85: browser integration, multi-node verification, and accepted regressions #89

Description

@lucas-barake

PR #85 rebuilt the relay as a cluster of per-device entities and deliberately scoped itself to the server core. This tracks what it left, in rough priority order. Each item says why it was deferred rather than done.

Browser integration

The relay has no browser consumer wired today. RpcPeerTransport composes with the new front door over RpcClient.layerProtocolSocket() and RpcSerialization, and test/RelayWireFormat.test.ts proves that composition over a real socket pair, but nothing connects ReplicaOwner / BrowserReplica to it and there is no browser websocket Socket layer. This is the largest remaining piece and the one that makes the relay usable end to end.

An end-to-end test of client custody against the real relay

RpcPeerTransport's custody logic — outbox replay, markCustody, receipt pruning, outer-digest validation — has never run against a real relay. Its own test fabricates the PeerRpc.RpcClient and the PeerRelayClientRuntime, and PeerRelayClientRuntime.test.ts uses the real runtime but a fake PeerSync. No test anywhere drives RpcPeerTransport or PeerSession.makeLive against a real PeerRelayClientRuntime or a real Sharding.

The composition exists — SqlReplica.layerRelay / DurableRuntime.layerRelay give real Sharding plus real relay-enabled PeerSync — but has zero test usage. Wiring one test through it would cover the seam that the whole "the frontend keeps working" claim rests on.

Multi-node verification

Single-owner-per-key across multiple runners is not verified. PR #85's harness is single-process (Runners.layerNoop + in-memory storage), so the property that makes the design correct is asserted by construction rather than by test. Needs a PostgreSQL-backed multi-runner harness with SqlMessageStorage, SqlRunnerStorage and RunnerHealth.layerPing.

Erase settled payloads

A terminal row keeps its stored envelope until retention deletes the whole row past deduplicate_until. The old relay erased the payload at the terminal transition and kept only deduplication evidence, so this is a data-minimization regression: an acknowledged message's bytes now sit in the relay database for the full retention window.

It is not fixable by erasing on every terminal transition — admit's revive path reuses the stored envelope in place so a sender that still holds custody can recover a DeadLettered or Expired message. Erasing only for Acknowledged and Rejected, which are never revived, would work.

Regressions accepted in #85 that may deserve a mechanism

Cross-inbox quotas. Per sender peer, per recipient peer, per recipient subject, per tenant and per shard caps are gone. Those counters span many inboxes so no entity is their sole writer, and enforcing them naively reintroduces the cross-process arbitration the cluster design removes. Admission is bounded per inbox only. A deployment needing tenant-wide caps needs a mechanism with its own single-writer story.

Connection and frame limits. maxRelayConnections, maximumRawChunkBytes, maximumDeclaredFrameBytes, maximumIncompleteFrameBytes and neighbours bounded the deleted bespoke framing. A single payload is still bounded by PeerRpc's schema check; concurrent connections and in-flight bytes are now unbounded by this package. Either re-home them onto the socket transport or decide explicitly that they belong to the deployment.

Deferred findings from #85's review rounds

Each was validated and consciously deferred rather than dropped.

  • A timed-out Settle reports ServerUnavailable after the durable write already landed, and the client's retry then gets SessionUnavailable rather than an idempotent "already settled". The recipient is left unsure whether its acknowledgement counted.
  • The store backoff is charged to the caller and to the whole entity lane. storeRetry is slept inside the ensuring stack, so the finalizer that answers a Settle only runs after it, and concurrency: 1 means Deliver / Heartbeat / EndSession queue behind it. Bounded and measured, but it belongs outside the recovery path.
  • The per-subject session cap is check-then-act across session-id minting and inbox-key derivation. It holds today only because NodeCrypto is synchronous, so the window is not schedulable — which is not a property to rely on.
  • The session finalizer's EndSession has no regression test. It is now bounded, and the mechanism is proven by an isolated probe, but the in-process harness cannot make an entity permanently unassigned, so no test distinguishes the fix. Needs a node-shutdown-level harness.

Smaller things

  • RpcPeerTransport.isRetryable has no caller and no test. Either wire it into a retry policy, cover it, or remove it.
  • Two known flakes, neither owned by the relay. packages/local-sql/test/fixtures/generate-fixtures.test.ts intermittently fails with EEXIST on the atomic-publish link in "leaves an interrupted publication as a recoverable version prefix". packages/local-test/test/Convergence.test.ts ("converges bounded concurrent list edits") intermittently times out at the 5s default under CI load. Both pass on rerun. Give each its own timeout or fix the temp-path collision rather than treating them as regressions.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions