pubsub/tasks/010-review-websocket-adapters.md

id, name, status, depends_on, scope, risk, impact, level

id	name	status	depends_on	scope	risk	impact	level
review-websocket-adapters	Review WebSocket client and server adapters	completed	websocket-client-tests websocket-server-tests	narrow	low	phase	review

Description

Review checkpoint after implementing both WebSocket adapters. These are the first fan-out adapter (server) and the first subscription-control adapter (client), so they establish patterns that the Worker and Iroh adapters will follow.

Verify:

• Subscription control protocol (__subscribe/__unsubscribe) is correctly implemented in both client and server
• Topic-based fan-out works end-to-end: client subscribes → server tracks → server fans out
• Backpressure protection works correctly
• Error handling matches the architecture spec
• Build, type-check, and test suite all pass

Acceptance Criteria

• npm run build passes cleanly
• npm run lint passes
• npm test passes with all core + Redis + WebSocket tests (181/181)
• WebSocket client subscription forwarding matches ADR-003
• WebSocket server fan-out and subscription tracking matches architecture spec
• No unnecessary comments in source (project convention)
• License headers present where needed

References

• docs/architecture/event-targets/websocket-client.md
• docs/architecture/event-targets/websocket-server.md
• docs/architecture/decisions/003-subscription-control-protocol.md

Review Report

1. Build / Lint / Tests

Check	Result
npm run build	Pass — tsup produces ESM + CJS + declarations for all 4 entry points
npm run lint (tsc --noEmit)	Pass — no type errors
npm test	Pass — 181/181 tests (46 ws-server, 36 ws-client, 25 redis, 10 redis-integration, 11 create_pubsub, 53 operators)

2. Client Adapter (event-target-websocket-client.ts) — Spec Alignment

Spec Requirement	Implementation	Status
Takes a single connected WebSocket	createWebSocketClientEventTarget<TEvent>(ws: WebSocket)	Pass
dispatchEvent sends JSON.stringify(event.detail) and returns true	Line 89: ws.send(JSON.stringify(event.detail)); return true;	Pass
addEventListener registers local listener + sends __subscribe on first listener per topic	Lines 38-56: addCallback tracks per-topic Set<EventListener>, sends __subscribe when isFirst	Pass
removeEventListener removes local listener + sends __unsubscribe when last listener removed	Lines 58-78: removeCallback deletes from Set, sends __unsubscribe only when callbacks.size === 0	Pass
null callback is a no-op (no listener registered, no __subscribe sent)	Lines 82-83, 92-96: if (callbackOrOptions != null) guard	Pass
EventListenerObject.handleEvent unwrapping	Lines 84, 95: "handleEvent" in callbackOrOptions ? callbackOrOptions.handleEvent : callbackOrOptions	Pass
Malformed JSON → silently ignored, warning logged	Lines 10-17: try/catch around JSON.parse with console.warn	Pass
Control events (__subscribe, __unsubscribe) from server → silently ignored	Line 19: envelope.type.startsWith("__") causes early return	Pass
Subscription reference counting (dedup)	Multiple addEventListener for same topic only sends one __subscribe (tested)	Pass
ws.send() failure → error propagates to caller	Line 89: dispatchEvent calls ws.send() directly, no try/catch — error propagates	Pass
Reconnection is caller-managed (new instance per connection)	Confirmed — no reconnection logic in adapter; tests verify new ws → new target → fresh __subscribe	Pass

3. Server Adapter (event-target-websocket-server.ts) — Spec Alignment

Spec Requirement	Implementation	Status
WebSocketLike interface (not raw WebSocket)	Lines 3-9: WebSocketLike with send, close, bufferedAmount, onmessage, onclose	Pass
SpokeEventTarget<TEvent> exposes ws property	Lines 11-13: SpokeEventTarget extends TypedEventTarget<TEvent> with readonly ws	Pass
addConnection(ws) / removeConnection(ws)	Lines 130-217, 98-128: Full lifecycle management	Pass
addConnection sets up onmessage/onclose, stores originals, calls onConnection	Lines 130-217	Pass
removeConnection cleans up subs/maps, restores original handlers, does NOT close ws	Lines 98-128: No ws.close() call	Pass
__subscribe control event → adds ws to topic's Set<WebSocketLike>	Lines 153-167	Pass
__unsubscribe control event → removes ws from topic set	Lines 170-186	Pass
Control events not dispatched to local listeners	Code returns early for __subscribe/__unsubscribe before local listener dispatch	Pass
Invalid topic in __subscribe (empty or non-string) → silently ignored	Lines 154-155: typeof topic === "string" && topic.length > 0	Pass
Duplicate __subscribe is idempotent (Set handles dedup)	Set<WebSocketLike>.add() is idempotent	Pass
Malformed JSON → caught, warned, message ignored	Lines 143-149	Pass
dispatchEvent → fan-out to subscribed connections + local listeners, always returns true	Lines 262-278	Pass
Per-connection spoke target: addEventListener/removeEventListener for events from that spoke only	Lines 47-96: createSpokeTarget with per-connection listener map	Pass
Per-connection spoke dispatchEvent sends to that specific ws	Lines 77-91: spoke.dispatchEvent checks backpressure then sends	Pass
Backpressure: check bufferedAmount > maxBufferedAmount before send, close with 1013	Lines 219-226 (sendToConnection) and 79-83 (spoke target)	Pass
Default maxBufferedAmount = 1,048,576 (1 MB)	Line 30	Pass
onBackpressure called before disconnect, cannot prevent it	Lines 221-222, 81-82	Pass
ws.send() failure → catches error, removes connection, fires onDisconnection	Lines 87-88 (spoke.dispatchEvent), 227-228 (sendToConnection)	Pass
Duplicate addConnection for same ws is a no-op	Line 131: if (spokeTargets.has(ws)) return;	Pass
onclose handler calls removeConnection and chains to original handler	Lines 209-214	Pass

4. ADR-003 Subscription Control Protocol Compliance

ADR-003 Requirement	Implementation	Status
Control events use EventEnvelope format	Both adapters send/receive `{ type: "__subscribe"	"__unsubscribe", id: "", payload: { topic: "..." } }`
id field is "" for control events	Lines 51, 72 (client)	Pass
Control events use payload.topic for routing, not type:id scoping	Client sends payload: { topic }, server reads envelope.payload.topic	Pass
__-prefixed types are reserved, not dispatched to user-facing listeners	Client: line 19 startsWith("__") → return; Server: lines 153-186 handle __subscribe/__unsubscribe with early return	Pass
Fire-and-forget semantics (no ack)	No acknowledgment mechanism in either adapter	Pass
Cleanup on disconnection	Server removes all subscriptions on onclose → removeConnection; Client per-connection so new instance needed	Pass
Reconnection requires new instance + re-subscribe	Client: each connection gets new instance, addEventListener triggers fresh __subscribe	Pass

5. Test Coverage

Client adapter (36 tests): Covers send path, receive path, topic scoping, subscription forwarding (including dedup), unsubscribe forwarding, malformed JSON, control event ignoring, envelope round-trip, null callback, EventListenerObject, reconnection, connection close, and subscription reference counting.

Server adapter (46 tests): Covers addConnection/removeConnection, automatic cleanup on close, subscription tracking (subscribe/unsubscribe/idempotent/invalid topic), topic-based fan-out, dispatched events to local listeners, per-connection spoke targets, incoming message aggregation, backpressure (threshold, callback, disconnect, below threshold, default 1MB), send failure handling, local addEventListener/removeEventListener, EventListenerObject, and onDisconnection callback.

All test items from the architecture spec test plans are covered.

6. Code Conventions

• No unnecessary comments in either adapter source file — Pass
• No stray debug code — Pass
• types.ts retains the graphql-yoga MIT license header — Pass

7. Exports

File	Status
src/index.ts — barrel re-exports createWebSocketClientEventTarget and createWebSocketServerEventTarget + types	Pass
tsup.config.ts — includes both event-target-websocket-client.ts and event-target-websocket-server.ts entries	Pass
package.json exports — includes both ./event-target-websocket-client and ./event-target-websocket-server sub-path exports with ESM/CJS/d.ts	Pass
No peer deps for WebSocket adapters (correct — WebSocket is web standard)	Pass

8. Minor Observations (non-blocking)

1. The client adapter uses the raw WebSocket type (browser global) rather than a WebSocketLike interface like the server. This is consistent with the spec ("No native deps — works in browsers and Node") but makes the client adapter harder to unit-test with alternative WebSocket implementations. The current tests use as any casts for mock WebSockets, which works but is less type-safe than the server's WebSocketLike pattern. This is a design choice, not a bug — the client is simpler (single connection) and the spec explicitly says "takes an already-connected WebSocket."

2. The server adapter has a subtle difference between sendToConnection (used for server dispatchEvent fan-out) and spoke.dispatchEvent (used for per-connection spoke sending). Both check backpressure and handle send failures, but the spoke target's dispatchEvent has its own inline backpressure check rather than calling sendToConnection. Duplicated logic, but functionally equivalent — no bug.

3. The server's onmessage handler checks typeof envelope.type !== "string" (line 151) but does NOT check for __-prefixed types beyond __subscribe and __unsubscribe. If a future control type like __ping were added, it would fall through to the regular event dispatch and be delivered to local listeners. This is fine as long as ADR-003's convention is followed — only __subscribe and __unsubscribe are defined currently. Future control types should be added to the handler.

Verdict

PASS — All acceptance criteria met. Both adapters correctly implement the architecture spec and ADR-003. Tests cover all specified behaviors. Build, lint, and 181 tests pass cleanly.

Summary

Review completed successfully. Both WebSocket client and server adapters fully align with their architecture specs and ADR-003. Subscription control protocol (__subscribe/__unsubscribe) is correctly implemented in both directions. Topic-based fan-out, backpressure protection, error handling, and connection lifecycle management all match spec. All 181 tests pass. Two minor non-blocking observations noted (client uses raw WebSocket type, duplicated backpressure logic in spoke target). No blockers found.