pneumo

M2: WebSocket hub — hello, replay, broadcast, heartbeat, revoke (ENG-254) #17

Merged

demi.dev 1w ago

targeting main from stoopkidddd/eng-254-m2-websocket-hub-hello-replay-broadcast-heartbeat-revoke

Merged opened by and assigned to

demi.dev 1w ago targeting main from stoopkidddd/eng-254-m2-websocket-hub-hello-replay-broadcast-heartbeat-revoke

Linear: https://linear.app/engineereddev/issue/ENG-254

What's in this PR:

ws/hub.ts: GET /api/ws upgrade on the same HTTP server (ws). hello within WS_HELLO_TIMEOUT_MS or 4401; the token is checked through the device registry's hash map (a revoked token fails at once); ready; replay from since (oldest first, ≤ 200) then replay_done; resync when since is unknown or > 200 behind; since: null → no replay. Any other frame, invalid JSON or a repeated hello → 4400.
Broadcast of push, push_deleted, device_added, device_updated, device_removed to every authenticated socket. On revoke, the event goes to the others first, then the device's sockets close with 4401.
Heartbeat: app-level ping every WS_PING_SECONDS; two missed pongs → terminate. Backpressure: bufferedAmount > 1 MB → terminate.
Presence: open-socket counts per device feed DeviceDTO.online (devices list, /api/me, events), with device_updated debounced 2 s.
Shutdown hook closes every socket with 1012. ws/timers.ts provides injected timers for fast tests.

Verify (lead, on 4f2605b, based on 555a737):

pnpm install --frozen-lockfile → exit 0
mise run check:server → exit 0 (Tests 215 passed (215), including 16 real-socket scenarios)
mise run e2e:server → exit 0 (14 tests; bundled runtime with the WebSocket on MinIO)
No pneumo-e2e-* containers left.

Deviations:

A repeated hello closes with 4400 (the spec says "any frame other than hello/pong after hello"; a second hello is treated as invalid).
Termination happens after two complete missed pong windows.

🤖 Generated with Claude Code

Commits 2

M2: Fix WebSocket hello deadline and payload limits (ENG-254)

+62 -7

M2: Add WebSocket delivery hub (ENG-254)

+1359 -42

Comments

v0 1w ago

demi.dev 1w ago

pneumo-reviewer: round 1, REQUEST_CHANGES #

The hub is mostly correct and well tested with real sockets: hello timeout, bad and revoked tokens, replay ordering and the 200/201 boundaries, resync, broadcast to 3 sockets, revoke ordering, heartbeat, presence, 1012 shutdown and 4400 errors. Presence feeds online in /api/devices, /api/me and device_* events. packages/ is untouched. Two fixes and one missing test before merge.

Findings #

FIX (often 2 × bad 7 = 14), ws/hub.ts:191 (with 150-158, 216-218): the hello timer stays armed while devices.authenticate() runs. That call can wait on an S3 lastSeen PUT (60 s throttle, no request timeout) and on the per-device queue. If authentication takes more than ~10 s, a client that sent a valid hello on time is closed with 4401, which §9 defines as "must not retry". Web signs out and Android stops its service, so a slow S3 moment signs users out. Reproduced with a probe. Fix: once a schema-valid hello is accepted, don't close 4401 on the deadline (cancel the timer or terminate()); only a null authenticate result → 4401. Add a real-socket test.
FIX (often 1 × bad 5 = 5), ws/hub.ts:45: WebSocketServer keeps the ws default maxPayload of 100 MiB. Before authentication, a 50 MiB text frame was fully buffered and parsed (close 4400, not 1009; ~250 MiB memory growth). A few unauthenticated sockets could exhaust a Railway instance. Fix: a small maxPayload (e.g. 64 KiB) and a 1009 test.
Missing acceptance test: "since: null → no replay" runs only against an empty push index, so a regression would pass. Seed a push first and assert exactly ready then replay_done.
FILE (3), ws/hub.ts:245-255: the 1012 hook waits for every close handshake, so one half-open peer holds shutdown until the 10 s deadline (exit 1). No data loss.
FILE (3), ws/hub.ts:135-140: a 200-frame replay of large pushes can exceed the 1 MB backpressure limit mid-replay. Client authors should advance the cursor per push.

FILE items are recorded on ENG-275; code left alone.

Spec checks #

§9 endpoint, frames, replay (oldest first, ≤ 200, replay_done), resync, heartbeat and close codes OK, except that the deadline-vs-auth case deviates (finding 1). §8 DELETE /api/devices/:id ordering OK (device_removed broadcast, then 4401). §12.7 1012 OK. §5.4 online in memory, never persisted, OK. §12.4 logging OK (deviceId + code only; token and envelope assertions present). Contract freeze OK.

DoD checks #

Injected/unref'd timers, real-socket tests on random ports, route tests for the presence changes, MinIO e2e (live push, revoke, 1012 on SIGTERM, replay after restart), hello-token redaction assertion: present. Missing: the null-cursor test (finding 3). README edit accepted.

Tests run by the reviewer: mise run check:server exit 0 (215 tests); mise run e2e:server exit 0 (14 tests); hub and device tests 5× with no flakes. → omp round 2 for findings 1–3.

v1 1w ago

demi.dev 1w ago

pneumo-reviewer: round 2, APPROVE #

Round 2 (053d82b) resolves both FIX findings and fills the test gap.

Findings #

Round 1 FIX (score 14), slow auth → 4401: resolved. session.cancelHello() now runs when a schema-valid hello is accepted. 4401 comes only from a null or revoked authenticate result; an exception still terminate()s (retryable). A new test holds auth past 10 s and asserts the socket stays open, then gets ready. The probe confirms it ("still open after deadline").
Round 1 FIX (score 5), maxPayload: resolved. 64 KiB; a 65,537-byte message (whole and fragmented) closes with 1009. The 50 MiB probe is rejected from the header without buffering.
Missing test, since: null: resolved. It now seeds two pushes and asserts ready, replay_done and nothing else.
FILE (often 1 × bad 1 = 1), ws/hub.ts:216-218: once a valid hello cancels the timer, nothing bounds a socket whose authenticate never settles and whose peer has vanished. It needs a real token (unknown tokens return null at once), and a live client reconnects on its own timeout. Optional: a generous auth bound that terminate()s.
Round 1 FILE findings (half-open shutdown, replay backpressure): unchanged, recorded on ENG-275.

Spec / DoD #

§9 hello deadline and 4401 now conform; since: null tested against history; the WS endpoint caps incoming messages. Everything else unchanged and OK. Contract freeze OK. Real-socket tests for slow auth, oversized frames and the null cursor added.

Tests run by the reviewer on 053d82b: mise run check:server exit 0 (218 tests); mise run e2e:server exit 0 (14 tests); hub tests 5× with no flakes. Pipeline for 053d82b: 3/3 green.

Versions

pneumo-reviewer: round 1, REQUEST_CHANGES #

Findings #

Spec checks #

DoD checks #

pneumo-reviewer: round 2, APPROVE #

Findings #

Spec / DoD #