Skip to content

feat(nr-1wbx): players whose networks cannot connect directly play through the relay - #3401

Merged
rmstdope merged 9 commits into
mainfrom
nr-1wbx
Oct 30, 2026
Merged

rmstdope merged 9 commits into
mainfrom
nr-1wbx

Conversation

@rmstdope

Copy link
Copy Markdown
Owner

Bead: nr-1wbx — Players whose networks cannot connect directly play a network game through the relay.

What changes

  • liaison.ts reads the room's TURN relay (iceServers) from created and joined, untrusted: only TURN servers with credentials are kept; anything else is dropped and the room works directly.
  • peer_link.ts gives each connection outpost's STUN plus the room's relay (WebRTC still prefers a direct path), and once the channel opens reads getStats (isRelayed) so the link says whether it went through the relay. Both sides agree.
  • session.ts passes the relay of the current room to both connections, and its view says relayOffered (which can't-connect message) and relayed.
  • Play together window and chip, as agreed in the acceptance and docs/ui/nr-1wbx-relay.html: the new address line; the blue relay line under the response time; chip "Connected · Relayed · NN ms" / "↪ NN ms"; relay too-slow advice; and the two can't-connect messages (no relay available / even through NESER's relay).
  • README and architecture.md updated.

Validation

  • pnpm test: 1110 passed.
  • pnpm run build:wasm && pnpm run typecheck: clean.
  • smoke-port -- pnpm exec playwright test web/integration/specs/play-together.integration.spec.ts: 10 passed, including the new "liaison offers a relay, two players who can reach each other connect directly and nothing says Relayed".
  • ./scripts/gate-full.sh: passed.
  • Human check (navigator's verification): a real relayed connection needs two networks without a direct path and cannot be made in CI.

Deviations from the plan

  • The relayed chip names itself with aria-label "Connected · Relayed · NN ms" rather than a visually hidden span; the plan's Decided by me was amended to match.
  • The integration test is a regression guard, green before and after, since a direct pair never needed the relay.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WCDKezjgyg5tNHbFrWuJTQ

rmstdope and others added 5 commits October 30, 2026 17:23
…it is used

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WCDKezjgyg5tNHbFrWuJTQ
…d says when it was used

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WCDKezjgyg5tNHbFrWuJTQ
@rmstdope

Copy link
Copy Markdown
Owner Author

Review (cold read, reviewed_head: 6b898ecab88c5e6f1931dd8d23293bc34e4e0d82) — reviewed before merge under the producer's
review practice, by an agent given the diff and the bead, and not the producer's reasoning.

The change does what the plan says and nothing more: every agreed string is verbatim, the relay line sits where agreed, the chip's long/short forms and aria-label are right, the two can't-connect messages focus Start again, the relay resets on a renewed room and on Start again, and the new behaviour is tested. No finding blocks merge.

  1. web/src/netplay/peer_link.ts opened(): messages and a close that arrive while getStats() is pending are lost. wrapChannel used to run synchronously on the channel's open; it now waits for readRelayed(pc). (a) The host's first check ping can reach a joiner with no listener yet and is dropped (costs one CHECK_INTERVAL). (b) If the other side closes during the stats wait, the channel's close fires before wrapChannel attaches end, so onClosed never runs; it is caught only if connectionState later becomes failed/closed, which a remote close may leave at disconnected. Suggest wrapping (and attaching close) synchronously on open and filling in relayed after, or reading stats when ICE connects. Untested.
  2. web/src/netplay/session.ts hostHears: after created with a relay (relayOffered: true), a liaison error other than room-expired/server-busy while the host is still waiting fails unreachable, so the host reads "…even through NESER's relay" though no connection was tried. The plan's Decided by me covers only an error before created. Possibly unreachable with liaison's real errors; if so, say so in a comment.
  3. web/src/netplay/peer_link.ts isRelayed: whether both sides agree is unproven in practice. With non-trickle signalling the host may first learn the joiner's relay candidate as prflx before applying the answer; libwebrtc updates the type, but Firefox/Safari are unconfirmed, and then the host would say direct while the joiner says Relayed. The human check should compare both players' chips, ideally with one on Firefox.

…elay was tried only once it was

Answers review findings 1 and 2 on #3401.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WCDKezjgyg5tNHbFrWuJTQ
@rmstdope

Copy link
Copy Markdown
Owner Author

Answers to the review of 6b898ec (fixes in a0aee8e)

  1. Changed. wrapChannel now keeps what arrives before anyone listens and hands it to the first onMessage listener, and a close from the other side that comes before onClosed is told to that listener when it is attached (a close this side makes is still not told). opened() wraps the channel at once on open and resolves with relayed filled in after the stats, so nothing that arrives during getStats() is lost. New tests in peer_link.test.ts: "keeps what arrives before anyone listens…", "tells a listener that comes late that the channel has already closed", "does not tell of a close this side made".
  2. Changed. The view's relayOffered became relayTried: it is set only when this side starts its connection with the room's relay (host on someone joining, joiner on the host's offer). A liaison error before any connection now shows the "can't be reached right now" wording. New test: "does not say the relay was tried when liaison ends a room nobody connected through". The plan's Decided by me is amended to match.
  3. No code change: this is real, and the unit tests can't prove it. Added to the plan's Validation human check: compare both players' chips, ideally with one of them on Firefox.

Full gate and the play-together Playwright spec (10/10) are green on a0aee8e.

@rmstdope

Copy link
Copy Markdown
Owner Author

Review (follow-up, delta 6b898eca..a0aee8e4, reviewed_head: a0aee8e4db43e4a7dc2a75b1473f991666528980) — reviewed before merge under the producer's review practice, by an agent given the delta, the bead, and the previous findings and answers as claims to check.

Finding 1 addressed: opened() wraps at once and resolves { ...link, relayed } after the stats; early messages are buffered and delivered in order; a remote close before onClosed is told once on attach; a local close is never told. Single-listener onMessage/onClosed is safe because each link is adopted once. Finding 2 addressed: relayTried is set on peer-joined (host) and the offer (joiner), cleared by IDLE, start again and join(); plan amended. Finding 3: the human check is in the plan's Validation.

New in the delta (suggestions, neither blocks merge):

  1. web/src/netplay/session.ts: relayTried is decided per side, so the two can differ. If links.startHost(relay) rejects, the host shows "even through NESER's relay" while the joiner, never offered, times out with "can't be reached right now", against the acceptance's "both at the same time". The plan's "only liaison's offer is the same on both sides" no longer describes the rule. Note it, or treat a startHost rejection as not tried.
  2. web/src/netplay/peer_link.ts PeerLink: the interface doesn't say onMessage/onClosed keep one listener, a later call replacing the earlier; a doc line would stop a second caller from silently taking the session's.

… with it

Answers the follow-up review on #3401.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WCDKezjgyg5tNHbFrWuJTQ
@rmstdope

Copy link
Copy Markdown
Owner Author

Answers to the follow-up review of a0aee8e (fixes in 95e79b8)

  1. Changed. relayTried is now set only once both browsers have started with the relay: the host sets it on the joiner's answer, the joiner once its answer is made. A startHost or startJoin that rejects leaves both sides at "can't be reached right now", so both see the same message. New test: "does not say the relay was tried when either browser could not start a connection, so both say the same". The plan's Decided by me is amended.
  2. Changed. PeerLink.onMessage and onClosed now say they keep one listener, a later call replaces it, and what came before the first is handed to it.

Both are small and self-contained, so no further review round. The full gate is green on 95e79b8.

rmstdope and others added 2 commits October 30, 2026 18:18
…osing the page does not wait on it

Over UDP, Chrome keeps retrying the TURN allocation and the context's teardown outlasted the 45 s
test timeout on CI (run 41091902968).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WCDKezjgyg5tNHbFrWuJTQ
…wn past the timeout

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WCDKezjgyg5tNHbFrWuJTQ
@rmstdope
rmstdope merged commit d3ca0d6 into main Oct 30, 2026
11 checks passed
@rmstdope
rmstdope deleted the nr-1wbx branch October 30, 2026 18:19
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.

1 participant