feat(relay): connection registry (#3)

[ilmoniemi]

What

Adds internal/relay/registry.go with:

• Conn interface (ConnID, Send, Close) — opaque connection handle for the routing core; real WS implementations land in relay: WS upgrade for /v1/server — accept binary connection, validate headers, claim server-id #4 / relay: WS upgrade for /v1/client — accept phone connection, look up server-id, register #5.
• Registry — a thread-safe store mapping server-ids to one binary (1:1) and to N phones (1:N). Single sync.RWMutex guards both maps.
• Methods: ClaimServer, ReleaseServer, RegisterPhone, UnregisterPhone, BinaryFor, PhonesFor (snapshot copy), Counts.
• Sentinel errors ErrServerIDConflict (4409) and ErrNoServer (4404).

Lock discipline: Send and Close are never invoked while the registry holds its mutex. ConnID is invoked under the lock by UnregisterPhone — the Conn doc comment names it as a non-blocking getter. PhonesFor returns a freshly-allocated slice so broadcast fan-out runs lock-free against the snapshot.

Issue

Closes #3.

Testing

internal/relay/registry_test.go (package relay, stdlib testing only):

• TestClaimServer_FirstWinsSecondConflicts
• TestReleaseServer_ReclaimSucceeds
• TestRegisterPhone_RequiresBinary
• TestUnregisterPhone_RemovesByConnID
• TestPhonesFor_SnapshotIsolation
• TestCounts_AcrossLifecycle (asserts orphan phones survive ReleaseServer per spec open-question 1)
• TestUnregisterPhone_RemovesEmptySlice
• TestRegistry_RaceFreedom — 32 goroutines × 200 ops hammering the public API

Verified locally:

• + "go vet ./..." + clean
• + "go test -race ./..." + clean
• + "go test -race -count=20 -run TestRegistry_RaceFreedom ./internal/relay" + clean
• + "go build ./cmd/pyrycode-relay" + succeeds

Architecture compliance

• Mirrors the doc-comment / sentinel-error idiom from + "envelope.go" + ; tests in package relay per established convention.
• Stdlib only (sync, errors in production; errors, fmt, sync, testing in tests).
• UnregisterPhone deletes the map entry when the slice empties (no orphan empty slices observable via Counts / PhonesFor).
• ReleaseServer is intentionally narrow — does not touch phones, leaving the grace period (relay: 30-second grace period on binary disconnect before releasing server-id #8) free to layer cleanup on top.
• No edits to doc.go, envelope.go, cmd/pyrycode-relay/main.go, or docs/ — registry has no consumers in this ticket.

🤖 Generated with Claude Code

[ilmoniemi]

Code Review: #3

Decision: PASS

Findings

None blocking. Implementation matches the architect's spec line-for-line.

Verification

• go vet ./... — clean
• go test -race ./internal/relay — clean
• go test -race -count=20 -run TestRegistry_RaceFreedom ./internal/relay — clean
• go build ./cmd/pyrycode-relay — clean

What I checked

Go correctness

• ClaimServer / ReleaseServer / RegisterPhone are atomic check-then-mutate under the write lock — no TOCTOU window for first-claim-wins or ErrNoServer gating.
• UnregisterPhone's swap-and-truncate (registry.go:135-137) handles the i == len-1 edge case correctly: self-copy, nil the last slot, truncate. GC-clears the freed reference per spec.
• PhonesFor returns a make+copy snapshot; broadcast fan-out can run lock-free against it. TestPhonesFor_SnapshotIsolation exercises both element overwrite and post-snapshot growth.
• Counts summed iteration is fine for a health-endpoint caller; not on the hot path.

Concurrency

• Single sync.RWMutex, no callbacks under the lock. ConnID() is the only Conn method invoked under the lock — documented contract on the interface (registry.go:27-30, 32-36) names it as a non-blocking getter.
• Send and Close are never invoked from registry methods — verified by reading every method.
• Race test (32 goroutines × 200 ops on 4 contended server-ids) hammers the public API and passes clean under -count=20.

Idiom & style

• Sentinel errors with errors.Is, mirroring envelope.go. Doc comments on every exported symbol name the concurrency contract.
• Stdlib only (sync, errors in production; errors, fmt, sync, testing in tests) — matches go.mod constraint.
• Tests in package relay per established convention.

Security (security-sensitive label)

• Architect's spec contains a ## Security review section with verdict PASS, so the design-level pass is on record. I walked the diff against the same checklist:
  • No tokens, secrets, or headers in log lines (no logging in registry at all — appropriate for a passive store).
  • No file I/O, no subprocess calls, no crypto, no network code in this diff.
  • Trust boundaries respected: registry treats serverID as an opaque map key (validation belongs at the WS upgrade layer, per spec). Per-server phone bound deferred to relay: WS upgrade for /v1/client — accept phone connection, look up server-id, register #5 per spec — documented in lifecycle expectations.
  • Lock-order graph is a single node; no deadlock possibility.
  • UnregisterPhone clears the freed slice slot to nil before truncation — no stale Conn reference held in the underlying array.

Test coverage

• All seven AC cases covered. Race test runnable manually via the documented go test -race -count=20 invocation; no count flag bled into the test code (correct — -count is a CI knob, not a test-source knob).
• TestCounts_AcrossLifecycle correctly asserts the orphan-phones-survive-release semantic from spec open-question 1.

Summary

Tight, well-documented implementation of a foundational data structure. Lock discipline is conservative and correct. Doc comments make the concurrency contract legible to future consumers (#4-#10). Ready to merge.