feat(relay): phone-side frame forwarder (#25)

[ilmoniemi]

What

Adds StartPhoneForwarder, the per-phone read pump that wraps each inbound phone frame in the routing envelope keyed by the phone's relay-assigned conn_id and writes the wrapped envelope to the binary holding the server-id. Replaces the placeholder c.CloseRead(...) + <-readCtx.Done() block in /v1/client with a real frame loop.

Adds WSConn.Read as the production-side phoneSource implementation; documents the single-reader contract on the type-level doc comment.

Issue

Closes #25.

Testing

internal/relay/forward_test.go covers four scenarios against a real Registry with package-private fakes (fakePhone for the read source, fakeBinary for the registry-side Conn with mu-protected sent):

• Forwards 3 frames bytewise — pushes 3 nested-JSON inner frames; asserts envelope ConnID and inner-frame opacity via json.Compact per the whitespace lesson.
• Phone disconnect mid-stream — closes the frames chan, asserts forwarder returns with io.EOF, then mimics the handler-level UnregisterPhone and asserts PhonesFor returns nil.
• No binary registered — forwarder logs phone_forwarder_no_binary and returns nil on the first frame; no panic.
• Context cancellation — forwarder returns within 100 ms of cancel() with context.Canceled.

Manual stress invocation is documented in forward_test.go's package doc per the race-count lesson:

go test -race -count=20 ./internal/relay/

Verified locally (-count=20 clean in 10s). go test -race ./... and go vet ./... pass.

Architecture compliance

• Follows the spec at docs/specs/architecture/25-relay-phone-forwarder.md:
  • phoneSource interface defined at the consumer (forward.go), not exported, not on WSConn — fakes substitute for tests; production passes *WSConn.
  • WSConn.Read is single-caller; does not plumb closeCtx because *websocket.Conn.Close already aborts in-flight reads.
  • Replaces CloseRead + <-readCtx.Done() entirely (does not retain it alongside the new reader — see lessons.md line 13-15).
  • The forwarder MUST NOT call UnregisterPhone or Close; the handler's existing defer owns cleanup.
  • Error handling matches the spec's table: phone Read err / Marshal err / missing binary / Send err — log + return.

🤖 Generated with Claude Code

[ilmoniemi]

Code Review: #25

Decision: PASS

Findings

• [NIT] internal/relay/forward_test.go:50,58-59 — fakeBinary.sendErr field is plumbed through Send but never assigned anywhere in the test file. The architect's spec didn't enumerate a Send-error test case (the four enumerated cases are all covered), but the dead field suggests an intended fifth case that was dropped. Either delete the field or add a quick test that sets bin.sendErr = someErr, pushes a frame, and asserts the forwarder returns it (the spec's error-handling table calls out this path explicitly: "binary.Send error → log, return err"). The grace-window scenario already exercises this path implicitly in production but not in tests.
• [NIT] PR description references the spec as docs/specs/architecture/25-relay-phone-forwarder.md, but the file was committed as docs/specs/architecture/25-phone-forwarder.md. Cosmetic.

Summary

Implementation is a faithful translation of the architecture doc:

• StartPhoneForwarder loop body matches the spec's pseudocode line-for-line; all four termination paths (Read err, Marshal err, missing binary, Send err) wired with the spec's log levels and return semantics.
• WSConn.Read correctly delegates to the wrapped *websocket.Conn without joining closeCtx — the architect's rationale (the underlying Close aborts in-flight reads) holds for nhooyr v1.8.x. Type-level doc updated to flag the single-reader contract; internal/relay/server_endpoint.go:98 still has the binary-side CloseRead placeholder, which is correctly left for the binary-side forwarder ticket.
• /v1/client handler call-site swap is clean: defer { UnregisterPhone; Close; log } ordering preserved, forwarder return value discarded with explanatory comment, no double-close.
• Tests are solid: real Registry + package-private fakePhone/fakeBinary (mu-protected sent), json.Compact canonicalisation per the whitespace lesson, generous-but-tight 100 ms cancel bound, manual stress invocation in the package doc.

Security review:

• Architect's ## Security review section in the spec is present with PASS verdict and a complete findings list — security-sensitive gate satisfied.
• Implementation matches the spec's security disposition. No tokens, headers, or frame bytes enter the four log call sites (server_id, conn_id, err only). The deferred SetReadLimit finding remains correctly out of scope (inherited from relay: WSConn adapter — nhooyr.io/websocket.Conn implementing the registry Conn interface #15; belongs in a follow-up against WSConn so the binary-side forwarder gets the same treatment).
• No new file/subprocess/crypto surface; concurrency posture unchanged (single goroutine per phone, RLock-only registry lookup).

CI: test + security both green.