Skip to content

Move an authorized Pocket session off the Relay onto a direct WebRTC channel - #613

Merged
nedtwigg merged 47 commits into
mainfrom
pocket-webrtc-2
Sep 11, 2026
Merged

Move an authorized Pocket session off the Relay onto a direct WebRTC channel#613
nedtwigg merged 47 commits into
mainfrom
pocket-webrtc-2

Conversation

@nedtwigg

Copy link
Copy Markdown
Member

After authorization, a Pocket session moves off the Relay onto a WebRTC data
channel — the same Noise session, the same counters, the same bounds, carried by
a direct connection between the two devices instead of by a server.

What a user gets

Latency. A keystroke stops making two hops through a relay that may be in
another region. On a LAN or a tailnet it goes straight to the laptop.

A Relay that cannot see the session. Relayed, it observes ciphertext sizes,
timing, volume, and specifically inter-keystroke timing. After the switch it
sees that the session exists and each end's liveness, and nothing else.

And a new way to lose a session, which is the honest cost. Once both
directions have switched there is no relay left to fall back to, so a channel
that dies ends the session: the phone leaves the wall and returning costs a
fresh handshake and one WebAuthn prompt. Before the switch a failed channel
costs nothing and the session stays relayed. The header names the live path and,
where it stayed relayed, which of three reasons it was.

Shape

Three layers, so the hardest invariant — preserving ciphertext order across two
transports — is stated once and testable on its own:

  • remote-lib-common/src/security/direct-path.ts — the four signals, their
    guard, the bounds, and DirectCutover, the per-direction state machine both
    ends run. Pure; knows nothing about RTCPeerConnection.
  • lib/src/remote/direct/direct-peer.ts — the RTCPeerConnection-shaped seam
    and one negotiation. Owns no policy.
  • lib/src/remote/direct/direct-endpoint.ts — one policy, two ends.

Every signal rides inside the session as a padded control message, so the
Relay never sees an SDP, a candidate, or that a direct path was attempted. No
ICE servers at either end: host candidates only, LAN and tailnet, and an
unreachable peer simply stays relayed.

Scope

The standalone Burrow answers, over node-datachannel's polyfill loaded at the
first offer and never at boot. VS Code declines — platform-targeted VSIX
builds are staged under ## Future rather than shipping ~92 MB of native
binaries in a universal package.

Review trail

This branch was compared against a competing implementation, then reviewed four
times: a four-angle quality pass, a high-effort correctness pass, an independent
Codex review, and a three-agent pass over everything the earlier reviews had
themselves produced. Findings that produced fixes, rather than notes:

  • A sender had no backpressure — a cat of a large file could overrun the
    runtime's send buffer, which on a switched session is burrow loss. Now a
    bounded queue with water marks and a drain.
  • A switched end waited on its peer only until the holding queue overran, which
    is a function of how chatty the session is. Now its own deadline.
  • Retiring a replaced session too early destroyed a healthy one on four failure
    paths, silently — a dismissed passkey prompt among them. Now it happens
    immediately before the one request that can race it.
  • A decline could be sent onto a session its own give-up had already ended.
  • The failure text an attempt gives up with — a runtime's exception string
    included — was one hop from a phone's screen. It is a closed cause set now,
    with the sentences owned by Pocket.
  • Two claims in the specs asserted enforcement that does not exist on the
    shipped stack. Both were measured, both are now stated accurately, and the
    measured behaviour is pinned so a future addon version is noticed.

Testing

pnpm test green: 2901 lib, 171 vscode-ext, 96 standalone, 251 website, 266
remote-lib-common, 275 relay, 123 dor, and every lint including the e2e
self-test.

The direct path is driven end to end in process through both real runtimes, and
over the real addon with a real Noise session on a real SCTP association.
scripts/direct-interop/run.mjs is a manual fixture for the one combination no
CI job can reach — a real browser negotiating against the real addon. Measured
on macOS 26.0 with Chromium and libdatachannel 0.24.3: the browser's offer was
585 characters against the 2 000-character bound, both ends reported a
262 144-byte association, and 65 535/4 096/33-byte frames crossed
browser→addon→browser byte for byte and in order.

Known gaps, deliberately not closed here

  • The latency claim is unmeasured. A tailnet dogfood with keystroke
    round-trip relayed vs. direct is staged in remote-api.md → Future.
  • Network transitions are untested on a real phone. The laptop ranks its LAN
    address above its tailnet address, so a session that forms on WiFi likely
    picks the LAN pair and would break on a walk to cellular; one that forms on
    cellular should survive the reverse, since the tailnet address does not move.
    Preferring tailnet candidates is the cheap fix if the walk confirms it — held
    until it does, because ICE may already fail over on its own.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ViV8RRXasxC1DMdNgqTKC8

nedtwigg and others added 30 commits September 9, 2026 17:48
Replaces remote-api.md's one-line Future item 8 with the staged design:
signaling inside the Noise session, host-candidates-only ICE, per-direction
cutover with bounded holding, the Relay kept as lifecycle authority.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
The four `direct-*` control shapes with their guard, the bound that keeps a
signal inside one control message, and the per-end `DirectCutover` that moves a
session off the relay in order (`docs/specs/remote-api.md` -> Future -> "Direct
path", stage 1). No endpoint uses it yet.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
`DirectPeerLike` / `DirectChannelLike` — the subset of the W3C API this stack
calls, so the browser's `RTCPeerConnection`, a native polyfill, and the linked
in-memory fake all satisfy one shape — plus `DirectPeer`, which owns one
negotiation, the single ordered channel, and the gathering and setup deadlines.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
The Client offers once the connection outcome says `ok`, the Burrow answers or
declines, each end's `direct-switch` is its last message on the relay, and every
byte after it is the same Noise transport message on the channel. Held frames,
the relay-after-switch violation, and every disposal path closing the peer come
from the shared cutover; the idle deadline and keepalives stay path-agnostic.

Pocket passes a factory over the browser's `RTCPeerConnection` with no ICE
servers and shows which path carries the session; `BurrowServiceOptions` carries
the option so a host can pass one, and none does yet.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
A real `PocketClient`, the in-memory Relay, and a real `BurrowRuntime`, each
holding one end of a linked fake peer pair: the signaling crosses the relay as
control messages, protocol-v1 and keepalives then cross the channel and the
relay sees nothing more, and every way it can end — decline, no factory, a setup
timeout, a dropped channel, a relay frame after the switch, an overrun holding
queue, `client-gone`, a dropped Burrow socket — lands where it should.

The relay harness gains `holdToClient`, the one ordering a synchronous
in-memory relay cannot otherwise produce, and `test-e2e-client` gains the Client
half of the negotiation for the two Burrow suites.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
`remote-api.md` gains a shipped `### Direct path` under Transport — the four
signals, the offer-once rule, the SDP bound, no ICE servers, the channel's
framing, the per-direction cutover and its bounds, and the Relay keeping
lifecycle authority — and its `## Future` item is cut to the four stages that
remain. `relay.md` names the signals as control messages and points here;
`pocket-app.md` says what Pocket offers, shows, and does when a channel dies.
Evidence for the candidate story and DTLS moved to the rationale.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
A setup deadline can fire on one end while the other's channel is opening, and
then that end's `direct-switch` lands on a peer with nothing left to receive it:
the Client's every request would hang unanswered, and the Burrow would hold the
session to its idle deadline. Both now treat that switch the way they treat a
channel that dies after one — burrow loss, and a fresh handshake to return.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
… text

A raw NUL in the source made git treat the whole test as binary.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
The sidecar builds its peer from node-datachannel's W3C polyfill, opened
inside the first `direct-offer` rather than at boot — a native library with
its own thread pool has no business loading for the sidecar starts that
never see a Client. A load that throws is warned once and declines forever
after: a missing platform package is a property of the installation, not of
the offer, and the answer is `direct-decline` either way.

Packaging: a sidecar package's transitive dependencies do not ship, because
the Tauri bundle copies only what is under `standalone/sidecar/node_modules`.
So the platform packages and `detect-libc` are declared there directly, and
both specifiers stay external to `burrow.cjs` — the addon resolves its
`.node` relative to its own `__dirname`, which inlining would move out of the
installed package. The build asserts the bare requires survived.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
`makeE2eHarness` and the account-plane fakes in front of it move out of
`pocket-client.test.ts` into `test-e2e-harness.ts`, beside the other shared
test modules, so a second suite can run the same real client, relay, and
Burrow — the reason `test-relay.ts` and `test-e2e-client.ts` are shared. No
behavior moves with it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
A Node↔Node case with the shipped node-datachannel polyfill on both ends:
real host candidates, real DTLS/SCTP, and the real Noise session on top —
the whole loop from `test-e2e-harness.ts` with only the peer swapped. It
pins the two things the in-memory pair cannot say anything about: that the
polyfill satisfies `DirectPeerLike` as written, and that one channel frame
carries a full `NOISE_MAX_MESSAGE_LENGTH` message intact.

A lost negotiation is retried rather than failed. Two agents in one process
occasionally settle on a candidate pair that answers ICE and then swallows
DTLS — every failure measured picked the same stray ULA IPv6 address, ~2% of
attempts — and staying relayed is what the protocol does about that.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
A native addon publishes one prebuilt package per platform and pnpm installs
only the host's, so the generator's walk of real directories threw on the
other five — the first dependency it has met that no single machine can
resolve whole. It now describes a platform package a *root* declares from
its installed sibling, which the family publishes in lockstep, so the
disclosure names every platform's package and reads the same wherever it is
generated. An unresolvable dependency outside that family still throws, and
a prebuild only the addon declares (android, musl) reaches no bundle and is
not listed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
remote-api.md names who answers instead of staging it, and the ledger loses
the item. standalone.md gains the packaging invariant the sidecar's Burrow
now depends on: a sidecar package's transitive dependencies do not ship, so
the addon's platform package and detect-libc are declared there directly and
both specifiers stay external to burrow.cjs. security-supply-chain.md states
how a prebuild for another platform is disclosed at all.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
The direct path is a transport change under an already-authorized session, so
the model spec now says so in one place instead of leaving a reader to infer it
from remote-api.md: same session and counters, signaling only inside the
ciphertext, offered only after promotion, DTLS beneath that nothing relies on,
never an ICE server, and a peer connection that outlives nothing. The UDP
socket ICE binds is named honestly — it is the one listener on every interface,
and its two parsers are attack surface — and Residual metadata now records that
a switched session stops exposing traffic timing to the Relay at all.

security-remote.md gains the eight audited rows, the network-posture prose for
that UDP socket (the loopback lint reads TCP bind spellings in our own source
and reaches nothing the addon binds), and a Relay-compromise row that says what
a switch takes away from it. Two of the rows are the prose the new e2e-lint
rules pin.

The direct-path bounds join the Burrow bounds table, evidence goes to the
rationale under `## Direct path`, and the ledger item this closes is deleted —
the supply-chain disclosure moves onto the item that declares the addon, where
CI already gates it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
Three rules, each with the self-test case that makes it load-bearing:

- No STUN or TURN URL in shipped source. Anchored on the quote that opens the
  literal, because `// early return:` and `Saturn:` both contain `turn:` and a
  rule that reddened on those would be deleted.
- No non-empty `iceServers` list. The empty array is the shipped value at both
  ends, so the pattern demands a first element.
- The Relay never names a direct-path signal or an SDP. Same reasoning as the
  protocol-v1 rule beside it: routing an opaque envelope needs none of these
  words, so one appearing is the leading indicator that a route has started to
  care what it carries.

`direct-path.ts` and `direct-peer.ts` join `E2E_MODULES`, so the curve rule
covers them. `direct-peer.ts` stays out of the optional-field rule's own list
on purpose — its `readonly sdp?: string` mirrors `RTCSessionDescriptionInit`,
whose optionality is the W3C API's; the signal that carries an SDP over the
wire keeps it required.

Verified by breaking each new pattern in turn: the self-test reports all three
as staying green when they should redden.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
The Traffic analysis entry enumerated what the Relay sees, and that list is now
conditional: an authorized session may move onto a direct connection between
the two devices, after which the Relay sees the session exists and nothing
about its traffic.

The staged-WebRTC check keeps its condition — `## Future` in remote-api.md
still names WebRTC, since the VS Code Burrow and relay-supplied ICE servers are
unbuilt — but its comment claimed the whole feature was below the fold. It is
half promoted, and the sentence the rule is still here to stop is one that
would be false for a VS Code user.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
A typed-array channel message was framed as a view over the event's buffer,
and the cutover may hold that frame until the peer's switch decrypts; the
send side already copied. Same shape on both sides now.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
Drop the `assertExternal` config key — it duplicated `external` (the same
`NATIVE_DIRECT` array), so it could never disagree with it. Assert from the
`external` list itself.

Check esbuild's metafile instead of grepping the emitted text: each external
specifier must appear in `metafile.outputs[<out>].imports` as
`{ kind: 'require-call', external: true }`. The old text grep matched
`require("…")` anywhere in the bundle, including inside an inlined module.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
`begin()` is the one-attempt-per-session gate and `abandon()` the one way
to give one up, so `onSwitchDecrypted()` can answer `fatal` for a switch
onto a channel this end abandoned rather than each endpoint tracking its
own `directAttempted` boolean beside the shared state.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
Replace the `@node-datachannel/` scope allowlist with the property it was
standing in for. `getDependencyNames` now yields `{ name, optional }`, and an
unresolvable dependency is resolved by the edge that declares it:

  - optional, from an external package — skipped, it ships to nobody
  - optional, from a product root — described from a sibling declared in the
    same `optionalDependencies` block at the same exact version string
  - anything else — still a hard error

The old rule described an absent prebuild from any installed package sharing a
scope prefix, which is a weaker guarantee than the lockstep publishing it
relies on.

Also fold both `externalPackages` key sites into `externalPackageKey`.

The generated disclosure is unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
Both endpoints re-implemented the same ~180 lines of cutover policy on
top of the shared state: the four channel handlers, the post-await
liveness re-check, abandon, the outbound fork, the drain loop. All of it
moves to `DirectEndpoint`, one per authorized session, constructed with
the four things the two ends actually differ in — who offers, how a
signal reaches the relay, how a ciphertext is decrypted, and what ends
the session.

`DirectPeer.send` answers false instead of throwing, so a refused send on
a switched session is burrow loss rather than an error thrown into the
relay path's contract. The Burrow's `#sendControl` carries a signal and
`#sendDirectSignal` is gone; the Client gets one of its own instead of
two inline try/catch blocks.

The handler cases move to `direct-endpoint.test.ts` over the fake peer
pair; the two endpoint suites keep the cases that prove the wiring.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
…orts

`DirectChannelLike.readyState` was never read and `DirectSignalType` had
no consumer. `DirectPeer.close` now drops the channel and swaps its
handlers for no-ops, so a closed peer holds neither the endpoint that
owned them nor whatever the channel was still holding — `#fail` reads
them first, since that is the one report a close owes.

The path indicator's labels gain the consumer that justifies the export:
one case that connects and asserts the label follows the client's report.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
One `FakeEventTarget` behind the fake socket, peer, and channel; one
`settle` from `test-e2e-client.ts` instead of a local tick loop; and the
frame accessors and the pair-approve-connect opening move onto
`E2eHarness`, where the native suite can reach them too. The direct-path
run gains an `answered()` waiter, so six copies of the same predicate
say what they are waiting for once.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
`resolveFrom` and `warn` existed only for the test, which now resolves
the polyfill itself and injects a plain factory — so the shipped factory
is two bare requires and a `console.warn`, exactly what the sidecar
bundle runs.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
…s them

Every pointer that named a symbol this refactor moved: the one-attempt
gate is `DirectCutover.begin`, the fatal-on-abandoned-switch is
`onSwitchDecrypted`, and disposal is `DirectEndpoint.dispose` on every
path that ends a session. `direct-endpoint.ts` joins the e2e boundary's
module list in the lint and in the audit prompt.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkBrKQZnhTvjcPCwmzUCcP
… stream

The receiver holds channel frames for one relay one-way hop — 100-300 ms to a
phone on cellular — and a PTY's ~1 KiB chunks reach the channel uncoalesced, so
64 frames overflowed in ~65 ms and overflow is fatal. Bytes are now the
operative bound at 4 MiB, with the frame cap raised past where 1 KiB frames can
reach it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NuqLYWrpEfQMmKLTPAfEpY
`DirectEndpoint.onRelayFrame` is now the single way a relay `transport` frame
reaches an authorized session: it refuses one that arrives after the peer's
switch, decodes the `ct` itself, and hands the receipt's control messages back
to its own signal dispatch. The decode moving inside is the fix — a `ct` that
passes the wire guard but will not decode was throwing out of the Client's
socket handler and being warned-and-dropped by the Burrow's drain, instead of
ending the session.

A failed decrypt on the Client is likewise `#loseBurrow` rather than
`#teardown`: what died is the end-to-end session, never the relay socket.
`#teardown` and `#loseBurrow` now share `#endSession`.

Also drops a why-clause from three rules that already carry a `(rationale)`
marker (AGENTS.md -> "Keying").

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NuqLYWrpEfQMmKLTPAfEpY
`DirectEndpoint.send` disposes the session when the channel refuses a
ciphertext, so a caller mid-message would route the remaining chunks onto the
relay of a session that no longer exists — post-switch ciphertext on the relay,
killing the peer with "relay frame after switch". Both `#sendApp` loops now
stop as soon as their session is no longer the live one, and a send on a
disposed endpoint answers `true` so nothing reaches the wire in between.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NuqLYWrpEfQMmKLTPAfEpY
`connect()` was overwriting `#established` and `#direct` without disposing what
they held, so a second Connect leaked the previous peer and channel — and
`onViolation`, alone among the channel handlers, was ungated, so a bad frame on
the orphaned channel called `fatal` on the session that had replaced it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NuqLYWrpEfQMmKLTPAfEpY
nedtwigg and others added 13 commits September 9, 2026 21:11
`binaryType = 'arraybuffer'` means both the browser and the polyfill dispatch a
fresh `ArrayBuffer` per message, so the copying branch in `#onMessage` was dead
and the live one was already a view. The only frame whose buffer has to outlive
its call is one the cutover holds, so that is where the copy belongs.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NuqLYWrpEfQMmKLTPAfEpY
…ng on

`close()` cancelled the setup timer but not the 3 s gathering one, and
`RTCPeerConnection.close()` fires no `icegatheringstatechange`, so a peer closed
mid-negotiation kept the suspended `offer()`/`answer()` frame, the endpoint and
the session alive behind a timer nothing would cancel.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NuqLYWrpEfQMmKLTPAfEpY
…the spec

`#answer()` carried its own copy of `offer()`'s staleness guard and of the
decline block; both are named now, and the `setTimer` spread that could never be
false is gone. `#promoteConnection` inlined `#disposeEstablished`'s body — which
it cannot call, since the prune would detach the state it is about to write —
so both go through `#clearEstablished`.

remote-security-model.md -> "Direct path" said the design was not restated and
then restated four of remote-api.md's rules; they collapse to one sentence of
pointers, and the dated candidate measurement moves to the rationale.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NuqLYWrpEfQMmKLTPAfEpY
Five safeguards the shipped cutover had no answer for, ported from the
competing `pocket-webrtc` branch and re-sized for terminal traffic.

A sender now bounds its own queue instead of handing the implementation
whatever the session produces: past DIRECT_BUFFER_HIGH the ciphertext
queues and drains at DIRECT_BUFFER_LOW, and once anything is queued
everything does, so nothing overtakes a frame encrypted before it. A
`cat` of a large file used to reach the runtime's own refusal, which on
a switched session is burrow loss.

Both queues are one DirectFrameQueue now, so "over the bound" means one
thing in both directions and the copy-on-queue rule is stated once.

A channel is checked before a session rides it — reliable, ordered,
this negotiation's label, and an association that can carry one whole
Noise transport message — and each failure abandons the attempt while
the relay is still carrying the session, rather than killing it later.

A switched end waits DIRECT_HANDOFF_TIMEOUT_MS for its peer's own
switch rather than until the hold overruns, which was a function of how
chatty the session happened to be.

A connection reporting `failed` or `closed` ends the attempt at once,
while `disconnected` is waited out for DIRECT_DISCONNECTED_GRACE_MS:
ICE reports it on gaps that recover, and after the switch ending one
costs a fresh handshake and a WebAuthn prompt.

Every route that gives an attempt up now says why, and Pocket carries
that reason in the indicator's hover text — never in the label, since a
session that quietly stayed relayed is still `relay` and a third state
for the common case would read as a fault.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ViV8RRXasxC1DMdNgqTKC8
The `*.node` sweep signs the addon's binary, but only a load proves the
hardened runtime lets the sidecar open it. Without this the first thing
to find out is a phone's first `direct-offer` on a user's machine —
where the answer is a silent `direct-decline` and a session that stays
relayed, which is exactly the outcome that looks like nothing is wrong.

Required through `node-datachannel/polyfill`, the specifier the sidecar
itself uses.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ViV8RRXasxC1DMdNgqTKC8
The two in-process suites cover the halves that can be faked: one links
two in-memory peers, the other runs the addon against itself. What
actually ships is a phone's browser stack negotiating with
node-datachannel in the sidecar, and no CI job has a browser — so that
pair had nothing behind it at all.

`scripts/direct-interop/run.mjs` serves the shipped `DirectPeer` over a
real `RTCPeerConnection` on a loopback page and answers it from the
addon, in the production direction: the browser offers, the Burrow
answers, `iceServers: []` at both ends. It is manual, gated by the
loopback guard and a per-run token, and it answers what a fake cannot —
whether a real browser's offer fits MAX_DIRECT_SDP_LENGTH, and whether a
real association carries a whole Noise message between the two stacks.

First run (macOS 26.0, Chromium, libdatachannel 0.24.3): the offer was
587 characters against the 2 000-character bound with one host
candidate, both ends reported a 262 144-byte association, and 65 535-,
4 096- and 33-byte frames crossed browser to addon and back byte for
byte, in order. The measurement and its caveat — that the SDP headroom
belongs to the host's interfaces, not to the code — are in the
rationale.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ViV8RRXasxC1DMdNgqTKC8
Review of the safeguards found the reason string had become product copy
without a boundary: `#giveUp` is reached from `#onClosed`, so every
string `DirectPeer.#fail` minted — a raw `String(error)` from a
description that threw included — was one sentence away from a phone's
screen, grammar-patched by the UI to make it read.

The transport now hands up a `DirectRelayCause`: unsupported, declined,
or failed. Pocket owns the sentence for each, beside every other Pocket
string, and the reason text stays what it always was — the operator's
log line. Adding a give-up path is now a compile error until someone
decides which of the three the user is told.

Also from the review:

- The queue keeps what it is given; `DirectCutover` copies, and only it.
  A sender's frames are ciphertext its session just minted and nothing
  else holds, so the copy was 4 MiB of pure waste exactly when the
  machine is already behind.
- `push` answers false instead of `wouldOverflow` then `push`, so there
  is no way to push past the bound.
- The peer reports its own failures: our queue overrunning and the
  channel refusing a write are opposite diagnoses, and the reason was
  the only thing telling them apart. The endpoint keeps one rule.
- The handoff deadline is derived in `#settle` rather than armed and
  cleared by hand at the four sites that already announce.
- `DIRECT_CHANNEL_LABEL` moved beside the other two-end constants: the
  answerer now validates a channel the peer created against it.
- `PocketClient.transportRelayCause` derives from the endpoint the way
  `transportPath` does, instead of mirroring it in a field nothing read.
- The interop fixture uses the shared `isAuthorized` gate, builds its
  two bundles together, and computes one verdict instead of two that
  could disagree. Re-run: still green, offer 585 characters.
- A channel defect can now sit on the answerer alone — the end where the
  check can fail in production — and `maxPacketLifeTime` has a case.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ViV8RRXasxC1DMdNgqTKC8
… stays

A high-effort review of the four safeguard commits found five things.

**An answerer whose channel is refused now declines.** `#adopt`'s new
validation reports through `onClosed` from inside `peer.answer()`, which
nulls the peer; `#answer` then found `#stillOurs` false and returned
without a word, leaving the offerer to wait out its whole 15 s setup
budget for a channel that was never coming — the exact thing `#decline`
exists to prevent. An end that has taken up an offer and not yet
answered it now declines from `#onClosed`, which also closes the same
hole for a negotiation that threw. That one predates these commits.

**The handoff deadline was measuring the relay this path exists to
escape.** Five seconds is a wager on a hop over a congested phone
uplink, and expiry is burrow loss while waiting costs only queue space —
which is separately bounded. It is now no shorter than the negotiation
before it.

**A new session no longer wears the previous one's reason.** A fresh
endpoint announces nothing until something changes, and its idea of
unchanged is `relay` with no cause, so the header kept explaining a
Burrow the phone had already left.

The other two were claims, not bugs, and the claims are what changed.
Measured against node-datachannel 0.33.2: the polyfill rebuilds every
incoming channel with its own defaults, so an offerer's
`{ordered: false, maxRetransmits: 0}` reaches the answerer as
`ordered: true, maxRetransmits: null`. The reliability half of the check
therefore reaches nothing on the standalone Burrow — only the label
does — and the addon exposes no other way to read what was negotiated.
The spec said it was enforced there; it now says what is true, calls the
check what it is (defence in depth against a paired Client, not a
boundary control), and a native case pins the behaviour so an addon that
starts reporting the flags is noticed instead of silently upgrading a
documented gap into an enforced rule.

The message-limit check has the same shape of caveat: the number is the
remote's advertised one, so it is per direction, and `sctp` is null on
both stacks until the association is up — so there is no earlier moment
to check and no way to decline before a peer may have switched. Stated
in the spec and the rationale rather than left implied by a comment that
said the attempt could always be abandoned.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ViV8RRXasxC1DMdNgqTKC8
The word already belonged to the migration off the plaintext relay
protocol — "the end-to-end cutover", five files on main across relay/,
vscode-ext/, lib/, and two specs, one of them a sentence e2e-lint pins
verbatim. The direct path arrived and started using it bare for the
relay-to-channel switch, so a reader hitting "after cutover" in
remote-security-model.md had two events to choose between and no way to
tell which.

The newcomer yields, since it is the newcomer and since it already owns
a precise word: `direct-switch` is on the wire, `switchOutbound` is in
the code, and "after the switch, nothing on the relay" is how the rule
already reads. Bare uses in the specs and the audit prompt now say the
switch, or name the policy rather than the moment.

`DirectCutover` keeps its name — it is qualified, and it is exactly the
per-direction state machine the word describes. The historical sense is
untouched, so the pinned lint sentence still matches. Comments inside
`lib/src/remote/direct/` are left alone too: the specs are the shared
vocabulary, a directory that is entirely one feature is not ambiguous.

One wording fix fell out: `DirectCutover.begin` answers true "once per
session", which is what its own doc says, not "once per cutover".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ViV8RRXasxC1DMdNgqTKC8
Three reviews over the previous commit — one of them of another agent's
work — found one regression, one latent bug of my own, and a test that
had already flaked once.

**The retire moved.** Retiring the previous session at the top of
`connect()` closed the race it was written for, but destroyed a healthy
session on four paths that used to leave it alone: a passkey prompt the
user dismisses, a denial, a handshake that throws, a malformed outcome.
`notifyGone: false` meant nobody was told. Verified against both
revisions — cancel the prompt and `hello()` went from working to
`'not connected to a burrow'`. Only the connection request can reach
`#promoteConnection`, so moving the retire to just above it closes the
same race and leaves every pre-flight failure harmless.

**A decline could land on a session that was already over.** A peer may
put `direct-switch` on the relay before it has been answered, which
`DirectCutover` accepts while the attempt is still `attempting`. Then
`#giveUp` took its fatal branch and `#decline` sent anyway — reaching a
Client that reads a refusal where the truth was a channel that never
opened, and, since the fatal branch leaves `#peer` set, doing it twice.
`#giveUp` now answers whether it abandoned, and only an abandoned
attempt is worth a signal.

**A dated measurement was wrong.** `sctp` is not null on the polyfill
before the association is up — the object is there from construction and
`maxMessageSize` is what reads null. The conclusion held, the fact
didn't, and the seam now types the field as nullable because the
shipped implementation returns null.

The native reliability case ran on the runner's default timeout with no
retry, in the one file whose comments say ~2% of local attempts lose
DTLS; it flaked in this session's own suite. It goes through
`untilOpen` now, like every other negotiation there.

Also: `TestRelay` grew `holdToClientWhen` / `isHoldingToClient` so a
case can hold a specific in-flight frame through the shared relay rather
than by rebinding its router — and the wait has to see the hold arm,
since releasing first leaves the predicate live with nothing to free it.
The `#disposeCeremony` in the `ok` branch stays, now saying why: a
concurrent Connect to another Burrow is the case the retire cannot see.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ViV8RRXasxC1DMdNgqTKC8
# Conflicts:
#	scripts/spec-word-budgets.json
@nedtwigg
nedtwigg marked this pull request as ready for review September 10, 2026 21:09
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 10, 2026

Copy link
Copy Markdown

Deploying mouseterm with  Cloudflare Pages  Cloudflare Pages

Latest commit: 14327ef
Status: ✅  Deploy successful!
Preview URL: https://6cafe608.mouseterm.pages.dev
Branch Preview URL: https://pocket-webrtc-2.mouseterm.pages.dev

View logs

@dormouse-bot dormouse-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

One finding. Everything else I traced held up — the per-direction cutover ordering, the disposal of the peer on every path that ends a session at both ends, the queue bounds in both directions, and the new e2e-lint rules with their self-test entries.

An abandoned attempt leaves no diagnostic anywhere, and abandonment is the outcome the design expects whenever the direct path does not form. #giveUp uses reason only on its fatal branch; on the abandon branch it is computed and thrown away, at both ends. So every cause that resolves to DirectRelayCause: 'failed' — an SDP over MAX_DIRECT_SDP_LENGTH, a negotiation that threw, a channel refused at adopt, an association under NOISE_MAX_MESSAGE_LENGTH, a channel that never opened, a signal the session refused — collapses into one sentence on the phone ("A direct connection was tried and did not work.") and nothing at all in the operator's log. The fatal branch is the only one that reaches console.warn, which is the opposite of where the ambiguity is: burrow loss has a reason, an attempt that quietly stayed relayed does not.

That asymmetry bites hardest on the SDP bound, because it is the one cause that is a property of the user's machine rather than of the network. MAX_DIRECT_SDP_LENGTH is derived from CONTROL_PAYLOAD_SIZE, so it cannot be raised without changing the padded control body, and the measurement behind it — docs/specs/remote-api.rationale.md -> "What a browser and the addon actually negotiate" — is explicitly "one host candidate on a machine with one usable interface". docs/specs/remote-security-model.md -> "Direct path" states that the addon advertises each routable interface at its port, so the Burrow's answer grows with interface count: a laptop carrying WiFi, Ethernet, a tailnet and a couple of Docker bridges, v4 and v6 apiece, is in the region where the answer stops fitting. The attempt is once per session and never retried, so for that user the direct path never forms on any session, and the evidence they can report is a sentence that reads identically to "the network would not route".

The inline suggestion is the one-line version: log the reason where it is currently discarded, so the SDP case is separable from the rest in the field. Whether the bound itself wants headroom is a separate call, but the log is what would tell you it needs one.

Comment thread lib/src/remote/direct/direct-endpoint.ts
`#giveUp` read `reason` only on the branch that ends the session, so the
rare outcome was logged and the common one was silent. A `DirectRelayCause`
is three buckets wide and is what the phone shows; the reason is the
sentence that says which failure it actually was, and "why is this
session still relayed" is the question an operator actually has.

Once per session at most — the attempt is never retried.

Found by dormouse-bot on #613.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ViV8RRXasxC1DMdNgqTKC8
`DirectEndpoint.send` answered a boolean and each end turned it into its
own relay fallback, with a `true` for a disposed endpoint standing in for
"drop it". The rule was spelled four ways across the endpoint and both
callers, and the two ends disagreed on what a replaced session does with
the relay half. It now takes a `sendRelay` dep and returns nothing:
direct once switched, relay until then, dropped once disposed — the
mirror of `onRelayFrame` on the way in. A `disposed` getter replaces both
ends' mid-message re-lookups, so `BurrowRuntime.#directFor` (a map walk
per PTY chunk) and `PocketClient.#deliver` are gone.

The Client's session record now carries its own endpoint and its own
routing triple, so no live session is ever without one and `#route` stops
minting an object per outbound chunk.

A session that ends announces `relay` with no cause from
`#disposeCeremony`, where the endpoint's lifetime is known, rather than
leaving `App.tsx` to reset the indicator on its way into the next
connect.

Alongside: the sidecar's `external` list is derived from the manifest
that installs those packages, and the metafile assertion checks that none
was inlined rather than re-checking the same hardcoded list; the remote
suites share the frame reader, the microtask drain, the poller and the
peer collector they had each re-spelled; and `fakeTimers` drops a
deadline on cancel instead of flagging it.

One behavior change: a message on an already-replaced session is dropped
rather than encrypted onto the relay, which is what the disposed case
already did.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@dormouse-bot dormouse-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Two findings on f843ff9d. The routing consolidation itself holds up: I traced the disposed-drops rule through both #sendApp loops, the Client's new per-session {route, direct} record against every path that sets or clears #established, and the switchOutbound idempotence claim against DirectPeer.#onOpen's if (this.#closed || this.#open) return; — the switch signal is sent before the cutover moves, so the old boolean was never the thing keeping a second switch off the wire, and the peer's guard is.

assertNothingInlined can no longer fail for the reason it exists. The old assertion was a presence check — each specifier had to appear as an external: true require-call edge — so a lost external entry made it go red. The new one is an absence check over SIDECAR_RUNTIME_DEPS, and that same list is what builds external. The two can now only be wrong together: move node-datachannel into optionalDependencies — where its own six @node-datachannel/* platform packages already live, so it is not a strange edit — and it drops out of external and out of names, esbuild inlines the addon, and the loop runs zero iterations and passes. external: [] is truthy, so even an empty derivation keeps the assertion wired up and vacuous. What ships is the burrow.cjs the docstring describes: one that "loads and then cannot find the addon's .node file, which nothing before the first direct-offer on a real machine would notice." The inline suggestion restores a loud failure for the one package the check is actually about; asserting the external require-call edge is still there, alongside the new inlined-input scan, would do the same job.

Related, whichever way that goes: docs/specs/standalone.md now reads "Every runtime dependency that manifest declares stays external", but the code reads dependencies only, and the six platform packages are declared under optionalDependencies. Nothing breaks — they are resolved by the external addon and never enter the bundle graph — but the sentence claims coverage the derivation does not have, so either it wants narrowing to dependencies or the derivation wants widening.

The second finding is the RELAYED_TRANSPORT comment; it is on the line.

Comment thread standalone/scripts/build-sidecar-proxy.mjs
Comment thread lib/src/remote/pocket-app/App.tsx Outdated
…eclaring the addon

`assertNothingInlined` scanned the same derived list that builds `external`,
so the two could only be wrong together: moving `node-datachannel` to
`optionalDependencies` — beside the six `@node-datachannel/*` platform
packages already there — dropped it from both, and the loop ran zero
iterations and passed on a `burrow.cjs` that cannot find its `.node`.
Guard the one package the check is about, so that edit fails the build.

`standalone.md` said every runtime dependency the manifest declares stays
external; the derivation reads `dependencies`, and the platform packages
are declared under `optionalDependencies`.

Also: `RELAYED_TRANSPORT` does not spare a render. The subscriber mints a
fresh object on every announcement, so `Object.is` never matches — it is
one value for the initial state and the default prop, and the comment now
says only that.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@nedtwigg

Copy link
Copy Markdown
Member Author

Both findings taken, in 14327ef3.

assertNothingInlined was vacuous. You're right that it can no longer fail for the reason it exists — deriving the assertion's names from the same manifest key that builds external means the two are only ever wrong together, and external: [] being truthy keeps the whole thing wired up and inert. I'd convinced myself the check was sound because I tested it by emptying the derived list and watched the build go red; that red was esbuild failing to resolve node-datachannel from the entry point's tree, not my assertion firing. Accident, not coverage.

Took the suggested guard. Confirmed it fires on exactly the edit you described — moving node-datachannel into optionalDependencies beside the six platform packages now fails the build with the guard's message rather than shipping a burrow.cjs that cannot find its .node.

Also narrowed the standalone.md sentence: the derivation reads dependencies, so claiming every runtime dependency was overclaiming while the platform packages sit under optionalDependencies.

RELAYED_TRANSPORT does not spare a render. Correct, and the comment was the wrong claim rather than a wrong constant — the setOnTransportChanged effect does setTransport({ path, cause }), a fresh object every time, so Object.is never matches and there is no bail-out to get. Took the suggested wording; the constant earns its place as one value for the initial state and the default prop, which is all it was ever doing.

On the earlier finding about #giveUp discarding its reason on the abandon branch — that shipped in a4b84c7f, before this pass.

@dormouse-bot

Copy link
Copy Markdown
Collaborator

Both landed as suggested, and the guard is in the right place: it runs at module top level, before bundles is built, so the failure is the build refusing to start rather than a bundle step reporting a clean metafile.

On the scope — one package rather than the derived list — I traced the other two dependencies entries against burrow.cjs's actual entry tree. detect-libc is resolved by the addon from its own node_modules, so an inlined copy in the bundle would not move what the addon loads; node-pty is never reached from remote/sidecar-entry.ts (the sidecar's unbundled main.js owns the PTYs), so it cannot be inlined there whatever external says. node-datachannel is the one entry whose __dirname-relative .node load is both reachable from this bundle and silent when it breaks, so guarding it alone is the right scope, not a remaining gap.

Approval stands; nothing further from me on this pass.

@nedtwigg
nedtwigg merged commit 5b3ae96 into main Sep 11, 2026
13 checks passed
@nedtwigg
nedtwigg deleted the pocket-webrtc-2 branch September 11, 2026 17:25
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.

2 participants