relay: binary-side frame forwarder (#26)

[ilmoniemi]

What

Adds StartBinaryForwarder, the per-binary read pump on /v1/server. Reads frames from the binary WS, calls Unmarshal on each, looks up the phone matching env.ConnID under the binary's serverID, and writes env.Frame (opaque bytes) to that phone. Replaces the c.CloseRead(r.Context()) + <-readCtx.Done() placeholder in server_endpoint.go.

Mirror of StartPhoneForwarder shape with one structural divergence: malformed envelopes, unknown conn_ids, and phone Send errors all log + drop + continue. The binary serves N phones, so a single bad frame or wedged sink must not tear the binary connection down. The forwarder returns only on binary.Read error or ctx cancel.

Issue

Closes #26.

Testing

Seven new tests in internal/relay/forward_test.go:

1. TestStartBinaryForwarder_RoutesToAddressedPhone — addressed phone receives the inner frame; siblings don't. Inner bytes byte-stable modulo whitespace via json.Compact.
2. TestStartBinaryForwarder_MultiplePhones — two phones, one envelope each, exact 1:1 routing.
3. TestStartBinaryForwarder_UnknownConnID_DropsAndContinues — bogus conn_id dropped; subsequent valid envelope still lands; forwarder still running (returns io.EOF only when frames chan closes).
4. TestStartBinaryForwarder_MalformedEnvelope_DropsAndContinues — []byte("not-json") dropped; subsequent valid envelope still lands.
5. TestStartBinaryForwarder_PhoneSendError_DropsAndContinues — p1.sendErr set; envelope to p1 dropped; envelope to p2 lands. Asserts the divergence from StartPhoneForwarder's return-on-Send-error.
6. TestStartBinaryForwarder_BinaryDisconnect_Returns — closing the source frames chan returns io.EOF; subsequent ScheduleReleaseServer(s1, 0) clears the slot — verifies handler-level defer behaviour structurally.
7. TestStartBinaryForwarder_ContextCancellation_Returns — ctx cancel causes prompt return.

fakePhone gained mu/sent/sendErr + Send/Close so it directly satisfies Conn. Existing #25 tests are unaffected (they use &registryConn{phone} and never inspect sent).

Local: go test -race ./... clean. go vet ./... clean. Manual stress: go test -race -count=20 -run TestStartBinaryForwarder_ ./internal/relay/ clean.

Architecture compliance

• Loop body matches the spec exactly: Read → Unmarshal → PhonesFor lookup → Send, with continue on the three drop branches and return on Read error.
• binarySource interface defined at the consumer (per the architect's preferred option), structurally identical to phoneSource but distinct named type for call-site clarity.
• server_endpoint.go call shape mirrors client_endpoint.go: _ = StartBinaryForwarder(r.Context(), reg, serverID, wsconn, logger). CloseRead removed in full per the lessons.md "WS handler that doesn't read" entry — the new loop is now the sole reader, and a parallel CloseRead goroutine would race the sole-reader contract on the underlying *websocket.Conn.
• Existing defer { ScheduleReleaseServer; Close; log } and defer cancelHB unchanged; LIFO unwind on forwarder return remains cancelHB → release → Close. The forwarder calls none of them.
• Logs enumerate fields explicitly (server_id, binary_conn_id, conn_id, err); no envelope/frame bytes or headers in logs.

🤖 Generated with Claude Code

[ilmoniemi]

Code Review: #26

Decision: PASS

Findings

None. The implementation matches the spec line-for-line and the architect's Security review section in docs/specs/architecture/26-binary-forwarder.md is present and dated 2026-05-10 (required for the security-sensitive label).

Verified:

• Loop body (internal/relay/forward.go:117-157) mirrors StartPhoneForwarder shape with the three documented divergences: malformed envelope, unknown conn_id, and phone.Send error all continue rather than return. Only binary.Read error returns. The structural rationale ("N sinks, one bad sink does not end the loop") is captured in the doc comment.
• server_endpoint.go:103-111 swaps CloseRead+<-readCtx.Done() for _ = StartBinaryForwarder(...), matching the client_endpoint.go shape. The CloseRead call is gone in full per docs/lessons.md § "A long-lived WS handler that does not read frames…". The handler's defer { ScheduleReleaseServer; Close; log } and defer cancelHB() are untouched; LIFO unwind remains cancelHB → release → Close. The forwarder calls none of them.
• No log leakage: the four log call sites (binary_forwarder_read_end, _unmarshal_err, _unknown_conn_id, _phone_send_failed) enumerate fields explicitly — server_id, binary_conn_id, conn_id, err. No envelope bytes, no env.Frame, no headers, no tokens.
• Cross-server addressing is structurally bounded: reg.PhonesFor(serverID) scopes the lookup to the binary's own slot; a forged env.ConnID cannot reach phones under a different server-id.
• Tests cover all seven AC bullets. fakePhone extended with mu/sent/sendErr + Send/Close per Approach (a) of the spec; existing #25 tests untouched (they wrap via &registryConn{phone} and never inspect sent). fakeBinarySource is a clean mirror of fakePhone's Read side.
• go test -race ./internal/relay/ clean. go vet ./... clean. go build ./... clean. Manual stress -race -count=20 documented in the test file's doc-comment header per the race-count lesson.
• Goroutine lifecycle: no new goroutines spawned by the forwarder; runs synchronously on the HTTP handler goroutine. All four termination paths in the spec (binary close, ctx cancel, heartbeat-driven close, server shutdown) terminate the single forwarder goroutine cleanly.
• Send-error divergence test (TestStartBinaryForwarder_PhoneSendError_DropsAndContinues) explicitly asserts the structural divergence from StartPhoneForwarder — p1 errors, p2 still receives.

Summary

Clean, narrowly-scoped implementation. Spec-faithful, no over-reach into the existing defer/heartbeat machinery, no error-handling improvisation, no logging of attacker-controlled payload bytes. The single fakePhone extension keeps test fakes minimal. Ready to merge.