relay: binary-side frame forwarder — unwrap routing envelope and dispatch to addressed phone

[ilmoniemi]

User Story

As the relay, I want a per-binary forwarding goroutine that reads frames from a binary WS, unwraps each routing envelope, looks up the phone matching the embedded conn_id, and writes the inner frame to that phone, so that binary-to-phone traffic flows after /v1/server upgrades complete.

Context

The binary-side data path. After /v1/server claims the server-id (#16), this ticket replaces the current c.CloseRead(r.Context()) + <-readCtx.Done() placeholder in internal/relay/server_endpoint.go with an actual frame loop. Frames are forwarded as opaque bytes — the relay MUST NOT parse the inner protocol envelope. See pyrycode/pyrycode/docs/protocol-mobile.md § Routing envelope.

This is the binary-side mirror of the phone-side forwarder shipped in #25. internal/relay/forward.go already exists; this ticket extends it with StartBinaryForwarder alongside the existing StartPhoneForwarder. The architect should mirror #25's established shape (synchronous despite the Start verb, returns error for observability, consumer-defined source interface so tests can substitute a fake) except where this ticket's AC explicitly diverges — see the Send-error AC below.

Acceptance Criteria

• In internal/relay/forward.go, export StartBinaryForwarder (signature shape per architect; conceptually (ctx, registry, serverID, binaryConn, logger) -> error, mirroring StartPhoneForwarder).
  • Reads frames from the binary conn in a loop.
  • For each frame: UnmarshalRoutingEnvelope (from relay: routing-envelope wrapper type — marshal, unmarshal, tests #1); resolve the addressed phone via Registry.PhonesFor(serverID) matching env.ConnID; call phone.Send(env.Frame) with the inner bytes.
  • Returns when the source conn returns a Read error OR ctx is cancelled.
  • Unknown conn_id: log a warning, drop the frame, continue the loop — phones come and go, this is expected. Do NOT close the binary.
  • Malformed envelope (Unmarshal returns a sentinel via errors.Is): log a warning, drop the frame, continue the loop.
  • Phone Send error: log + drop the frame, continue the loop. The owning phone handler will see its own read fail and clean up. This diverges from StartPhoneForwarder's return-on-Send-error behaviour: the binary is still serving other phones, so a single bad sink does not end the loop.
• /v1/server handler in internal/relay/server_endpoint.go calls StartBinaryForwarder after ClaimServer returns nil and the heartbeat goroutine has been launched, replacing the current c.CloseRead(r.Context()) + <-readCtx.Done() placeholder. The CloseRead call MUST be removed — it drains-and-discards data frames, which would race the forwarder reader. Mirror the wiring shape used in client_endpoint.go for StartPhoneForwarder (_ = StartBinaryForwarder(r.Context(), reg, serverID, wsconn, logger)). The HTTP handler returns when the forwarder returns; the existing defer { ScheduleReleaseServer; Close; log } (relay: WS upgrade for /v1/server — accept binary connection, validate headers, claim server-id #16/relay: /v1/server — defer binary release by 30s grace window #21) and defer cancelHB (relay: WebSocket heartbeat — RFC 6455 ping/pong every 30s, close at 60s #7) run after under LIFO order. The forwarder MUST NOT call any of those itself.
• Tests in internal/relay/forward_test.go (extending the existing file from relay: phone-side frame forwarder — wrap inner frames in routing envelope and send to binary #25; mock binary+phone conns wired to a real Registry):
  • Binary sends an envelope addressed to a known conn_id: that phone receives the inner frame; other registered phones do not. Inner frame is byte-stable round-trip — canonicalise via json.Compact per the whitespace lesson.
  • Multi-phone: 2 phones registered; binary sends one envelope per phone; each receives only its own.
  • Unknown conn_id: log warning, frame dropped, no panic, binary forwarder continues, other phones unaffected. Assert continuation by sending a valid envelope after the bad one and observing it land.
  • Binary disconnects (mock Read returns an error): forwarder returns; the handler-level defer runs ScheduleReleaseServer as expected.
  • Context cancellation: forwarder returns promptly.
• make test clean with -race. Document go test -race -count=20 ./internal/relay/ in the test file's doc comment, per the race-count lesson.

Technical Notes

• Inner frames are opaque. Test envelopes should carry inner frames containing nested JSON; assert byte-stable forwarding via json.Compact on both sides.
• A single bad frame from the binary MUST NOT tear the binary connection down. Phones come and go; an envelope addressing a just-disconnected phone is a normal race, not a binary fault. Log and drop, never close — this is structural.
• Sentinel errors from UnmarshalRoutingEnvelope (relay: routing-envelope wrapper type — marshal, unmarshal, tests #1) branch via errors.Is; the established pattern is in place.
• The handler's defer { ScheduleReleaseServer; Close; log } (relay: WS upgrade for /v1/server — accept binary connection, validate headers, claim server-id #16/relay: /v1/server — defer binary release by 30s grace window #21) and heartbeat (relay: WebSocket heartbeat — RFC 6455 ping/pong every 30s, close at 60s #7) handle cleanup and liveness; the forwarder is purely a read-pump.
• Out of scope: the phone-side forwarder (shipped in relay: phone-side frame forwarder — wrap inner frames in routing envelope and send to binary #25); heartbeat (shipped in relay: WebSocket heartbeat — RFC 6455 ping/pong every 30s, close at 60s #7).

Size Estimate

S — one new forwarder in an existing file + one handler call-site swap (remove CloseRead+<-Done, add StartBinaryForwarder call) + 5 test scenarios. ~60 LOC production + ~140 LOC tests.

Split from #6.

[ilmoniemi]

🤖 Product Owner — creates structured issues

po agent has completed work on this ticket.

Agent output (click to expand)

The ticket #26 has no `needs-rework:po` label and zero comments — the harness's "rework" hint appears to be a default mode flag rather than a real downstream signal. Auditing the body against the PO contract:

- **User Story / Context / AC / Technical Notes / Size** — all present and well-shaped
- **Single concern** — binary-side forwarder only; phone-side is the sibling #25
- **Testable AC** — each test scenario specifies inputs and observable outcomes
- **No prescribed signatures** — body says "signature shape per architect"
- **Lessons captured** — references `json.Compact` whitespace lesson and `-race -count=20` doc-comment lesson; sentinel-error pattern via `errors.Is`
- **Cleanup contract explicit** — forwarder MUST NOT call `ScheduleReleaseServer` / `Close`; handler defer (from #16/#21) owns those
- **Mirror coordination with #25** — body covers either landing order ("Whichever lands first creates `forward.go`; the second extends it")
- **Size:s justified** — ~60 LOC + ~140 LOC; doesn't trip any quantitative red line (3 files max ✓, 1 file touched + 1 new + 1 test = 3, ≤5 AC ✓ with sub-bullets describing one forwarder's behavior contract, no "and" in the title)
- **Labels** — `security-sensitive` (frame routing on internet-exposed surface) ✓, `size:s` ✓
- **Prerequisites** — #1, #16, #21 all Done per PROJECT-MEMORY

No edits needed. Body is engineering-ready; leaving it untouched so the dispatcher auto-adds `ready:po` and advances to architect.

Ready for human review. Move to the next column when approved.

[ilmoniemi]

Architect — blocked on file-overlap, routing back to PO.

Two in-flight branches modify files this spec must touch. Either would produce a structural merge conflict at integration time.

• relay: phone-side frame forwarder — wrap inner frames in routing envelope and send to binary #25 (wip:code-review, mirror-image phone-side forwarder) — creates internal/relay/forward.go and internal/relay/forward_test.go. The body of relay: binary-side frame forwarder — unwrap routing envelope and dispatch to addressed phone #26 itself acknowledges the dependency: "whichever lands first creates internal/relay/forward.go; the second extends it." These two are designed to serialize, not run in parallel.
• relay: WebSocket heartbeat — RFC 6455 ping/pong every 30s, close at 60s #7 (heartbeat, branch pushed) — inserts a per-conn heartbeat goroutine in internal/relay/server_endpoint.go immediately above the <-readCtx.Done() block that relay: binary-side frame forwarder — unwrap routing envelope and dispatch to addressed phone #26 must replace with the forwarder call. Same region → guaranteed conflict.

blockedBy set to #25 and #7. Once both close, this ticket auto-advances back to In Architecture and the next architect run starts from current main — no stale-branch merge.

No spec written; worktree untouched.