[max_turns] relay: WebSocket heartbeat — RFC 6455 ping/pong every 30s, close at 60s

[ilmoniemi]

Auto-salvaged from max_turns

The developer agent hit max_turns (71 turns, $6.14) on #7 while work was in progress. The dispatcher auto-committed the uncommitted changes and opened this draft PR for human triage.

Build status at salvage: clean (go vet ./... + go build ./... all passed). Tests were not run as a salvage gate — failing tests are often the signal the agent was chasing.

Last messages from the agent (may include unresolved findings):

To investigate:

• Resume the session: claude --resume c7fc64e5-8fd9-4291-93c3-62c82b012286
• Branch: feature/7
• Issue: relay: WebSocket heartbeat — RFC 6455 ping/pong every 30s, close at 60s #7

This PR is a draft — auto-merge is disabled until a reviewer marks it ready (or closes it). Ticket label error:max_turns_salvaged indicates triage required.

Closes #7

[ilmoniemi]

Code Review: #7

Decision: PASS

Findings

None blocking.

Notes (informational)

• internal/relay/heartbeat_test.go:82 — teardownOnce is a non-atomic bool rather than sync.Once. Safe in practice because each test calls teardown only via defer from the test goroutine; documenting in case future tests want to call it from multiple sites.
• internal/relay/heartbeat_test.go:48 — serverWS is set inside the handler goroutine and read in the test goroutine; the <-handlerReady channel after close(handlerReady) provides happens-before, so this is race-free (and -race confirms). Worth knowing if the helper grows additional shared state later.

Summary

Implementation matches the spec one-to-one. The defer ordering is correct in both endpoints (cancelHB registered after the release/unregister defer → runs first under LIFO → heartbeat exits via <-ctx.Done() before wsconn.Close() fires); runHeartbeat's structure has exactly one CloseWithCode call site gated on ctx.Err() == nil, so AC bullet (c) is enforced structurally. WSConn.Close and WSConn.CloseWithCode correctly share the same closeOnce, which preserves the single-close-frame-on-the-wire invariant.

Security-sensitive checklist: spec carries a PASS security review section (architect's pre-implementation pass); the diff introduces no tokens/secrets/log leaks (close reason is the fixed literal "heartbeat timeout"), no new untrusted-input boundary (heartbeat operates post-claim), bounded concurrency (1 goroutine / accepted upgrade), bounded write rate (server-driven ticker), bounded write size (library-default empty ping payload), and defer cancelHB() is unconditional once the goroutine has been launched.

go vet, go test -race ./..., and make build all pass locally.

Knowledge artefacts (ADR-0007, features/heartbeat.md, INDEX.md and PROJECT-MEMORY.md updates) are present and substantive. The architect's "logging on heartbeat-induced close" open question was deliberately not adopted, with a justification that aligns with the project's evidence-based pacing rule — a good call.

The salvaged pyrycode-relay binary was removed and .gitignore was updated to prevent recurrence; nice to see that closed out as part of the same PR.