Main

.env.example

@@ -3,3 +3,4 @@ WS_HEARTBEAT_MS=30000
 MAX_PAYLOAD_BYTES=1024
 DATABASE_URL=
 AUTH_SECRET=change-me-to-a-random-secret
+MAX_TIMESTAMP_SKEW_MS=30000

issues/issue-01-reconstruct-scrambled-server.md

@@ -0,0 +1,72 @@
+## Title
+Reconstruct the fatally corrupted `server.js` message pipeline — duplicate exports, scrambled control flow, and undefined variable references
+
+## Difficulty
+10/10 — Expert. Estimated effort: 3–5 days for a senior engineer.
+
+## Context
+`src/server.js` is the central nervous system of the entire gateway. It is currently in a catastrophic broken state: it contains **two** `export function createServer()` declarations (lines 9 and 135), scrambled function bodies where variables are referenced before declaration, and references to at least three completely undefined identifiers (`rateLimiter`, `ipConnectionCount`, `req` inside `handleMessage`). Because of the duplicate export, **every test suite that imports `createServer` fails at parse time** with `SyntaxError: Identifier 'createServer' has already been declared`. This means 8 of 18 test suites (`server.test.js`, `binary-frames.test.js`, `conn-rate-limiter.test.js`, `max-connections.test.js`, `index.test.js`, `integration.test.js`, `message-size-limits.test.js`, `validator.test.js`) cannot even load, let alone run.
+
+This is not a refactor. This is a reconstruction of a broken module using the surviving test expectations and the other working modules as behavioral contracts.
+
+## Problem statement
+Reconstruct `src/server.js` so that it exports exactly one `createServer` function which:
+
+1. Binds a `WebSocketServer` on the configured port with `maxPayload`.
+2. On each connection: extracts the IP from `req.socket.remoteAddress`, checks the per-IP connection rate limiter (`createConnRateLimiter`), parses the auth token from the query string, and calls `verifyConnection(token)`. If auth fails, closes with code 4001. If connection rate limit is exceeded, closes with code 4029.
+3. Resolves the effective `clientId` from the auth result (`authResult.clientId ?? clientId` where `clientId` is the uuid generated per connection).
+4. Sets up per-message rate limiting using `createRateLimiter` (imported from `./rate-limiter.js`), checked on every incoming message before validation.
+5. Validates each incoming message via `validateMessage` from `./validator.js`. On validation failure, sends `{ type: "error", payload: { message: "<error>" } }` back to the client.
+6. Routes validated messages through the room manager: `join_room`, `leave_room`, and `location_update` (fan-out to all rooms the client belongs to, excluding the sender).
+7. On disconnect: calls `rooms.disconnect(actualClientId)` and decrements the per-IP connection count.
+8. Sets up heartbeat ping/pong via `setupHeartbeat` that terminates zombie connections (`ws.isAlive === false`).
+9. Returns `{ wss, rooms }` (not `ipConnectionCount`).
+
+The scrambled code currently interleaves fragments of what was clearly a `handleConnection` function and a `handleMessage` function in the wrong order, with the `handleMessage` body referencing `req` which was only available in the connection handler scope.
+
+## Current behavior
+`src/server.js` has:
+- **Line 9**: First `export function createServer({ port, heartbeatMs, maxPayloadBytes, connRateLimit } = {})` — creates WSS, rooms, and connRateLimiter, defines `safeSend` and a scrambled `handleMessage`, then falls through to undefined variable references.
+- **Line 26**: `function handleMessage(ws, clientId, rooms, raw)` — but the body references `req` (line 29), `ip` (line 30), `connRateLimiter` (line 31), `url` (line 37), `token` (line 46), `authResult` (line 47), `rateLimiter` (line 84, never defined anywhere), and `actualClientId` (line 92, assigned after first use on line 84).
+- **Line 135**: Second `export function createServer({ port, heartbeatMs, maxPayloadBytes } = {})` — a cleaner but incomplete implementation that wires up `wss.on("connection", ...)` and `setupHeartbeat` but doesn't handle auth, rate limiting, or message routing.
+- **Line 151**: Returns `{ wss, rooms, ipConnectionCount }` where `ipConnectionCount` is never declared.
+
+Additionally, `createRateLimiter` from `./rate-limiter.js` is never imported. The variable `rateLimiter` referenced on line 84 is undefined. The variable `ipConnectionCount` referenced on line 100 and returned on line 151 is never declared.
+
+## Required behavior
+- Exactly one `export function createServer()` that accepts `{ port, heartbeatMs, maxPayloadBytes, connRateLimit }`.
+- Imports `createRateLimiter` from `./rate-limiter.js`.
+- Implements the full connection lifecycle described in the problem statement.
+- All 8 currently-failing test suites pass.
+- No references to undefined variables.
+- `safeSend` is defined and used for all outbound messages.
+
+## Constraints
+- Do not change the public API of `createServer` beyond adding the optional `connRateLimit` parameter (already tested).
+- Do not change any file other than `src/server.js`.
+- Do not add new npm dependencies.
+- Do not modify any test file.
+- The `handleMessage` function must be properly scoped — it should receive `ws`, `clientId`, `rooms`, `raw`, and the rate limiter instance (or a closure over it), but NOT `req`.
+- Auth check (`verifyConnection`) must use the token extracted from `req.url` query params, not from `raw`.
+
+## Acceptance criteria
+- [ ] `src/server.js` exports exactly one `createServer` function
+- [ ] No `SyntaxError` when importing `src/server.js`
+- [ ] `npm run lint` passes with zero errors
+- [ ] `npm test` passes: all 18 test suites green (166+ tests)
+- [ ] `server.test.js`: connection with valid token accepted, missing token rejected with 4001, invalid JSON returns error frame, join_room returns room_joined, leave_room returns room_left, location_update broadcasts to room members, sender excluded from own broadcast, disconnect cleans up room membership
+- [ ] `binary-frames.test.js`: binary Buffer frames accepted for join_room and location_update, invalid JSON in binary frame returns error
+- [ ] `conn-rate-limiter.test.js`: connections exceeding per-IP rate limit rejected with 4029
+- [ ] `max-connections.test.js`: connections exceeding per-IP max rejected with 4029
+
+## Out of scope
+- Changes to `auth.js`, `validator.js`, `room-manager.js`, `rate-limiter.js`, `conn-rate-limiter.js`, `logger.js`, or any test file.
+- Adding new message types or protocol features.
+- Database integration.
+- TLS or HTTP server setup.
+
+## Hints and references
+- The test file `tests/server.test.js` defines the exact behavioral contract: `connect()` helper builds `ws://localhost:${port}/?token=${token}`, `nextMessages()` collects N messages, `closeAll()` tears down sockets. Study these helpers to understand the expected protocol.
+- The variable `clientId` should be generated with `uuid.v4()` (already imported but unused in the second `createServer`).
+- The `rateLimiter` (per-message) should be a module-level `createRateLimiter()` instance, not per-connection, since the rate limit is per-client-id and the same client ID could theoretically reconnect.
+- The `ipConnectionCount` tracking in the close handler (lines 98-108 of current file) is for a max-connections-per-IP feature. The test `max-connections.test.js` expects a `maxConnectionsPerIp` option. You must implement this as an in-process Map tracking active connection count per IP, incremented on connection and decremented on close.

issues/issue-02-reconstruct-validator-pipeline.md

@@ -0,0 +1,61 @@
+## Title
+Reconstruct the corrupted `validator.js` — resolve duplicate declarations and undefined helper references to restore the dual-input validation pipeline
+
+## Difficulty
+9/10 — Expert. Estimated effort: 2–3 days for a senior engineer.
+
+## Context
+`src/validator.js` has two competing `export function validateMessage()` declarations (lines 33–41 and 63–84). Because ES modules do not allow duplicate exports of the same name, this module fails at parse time with `SyntaxError: Identifier 'validateMessage' has already been declared`. This breaks every test suite that imports the validator: `validator.test.js`, `integration.test.js`, `message-size-limits.test.js`, and transitively `server.test.js` (which imports `validateMessage` indirectly through `server.js`).
+
+The first definition (lines 33–41) is incomplete — it parses JSON but never invokes the Zod schema, never returns a result for valid input, and doesn't enforce per-type byte size limits. The second definition (lines 63–84) is more complete but references three functions that do not exist anywhere in the file: `parseJSON` (line 64), `isString` (line 67), and `formatIssue` (line 80). A standalone `buildError` helper exists (lines 49–51) but is never called — `formatIssue` is used instead.
+
+## Problem statement
+Reconstruct `src/validator.js` into a single, correct validation pipeline that:
+
+1. Accepts either a raw string (from WebSocket `Buffer.toString()`) or a pre-parsed object (from tests that call `validateMessage` with an object directly).
+2. Parses JSON from strings; passes objects through.
+3. Enforces per-type byte size limits **only** when the input is a string (binary frame → string). The limits are defined in `MESSAGE_SIZE_LIMITS`: `location_update` ≤ 512 bytes, `join_room` ≤ 256 bytes, `leave_room` ≤ 256 bytes.
+4. Validates the parsed object against `messageSchema` (the Zod discriminated union).
+5. Returns `{ ok: true, data }` on success or `{ ok: false, error }` on failure.
+6. Exports `validateMessage` exactly once and exports the `buildError` helper.
+
+## Current behavior
+- **Line 33**: First `validateMessage(raw)` — calls `JSON.parse` on strings, but the function body ends at line 41 without returning a validation result or calling the Zod schema.
+- **Line 63**: Second `validateMessage(raw)` — calls `parseJSON(raw)` (undefined), checks `isString` (undefined), uses `formatIssue` (undefined) instead of the existing `buildError` helper.
+- **Line 49**: `buildError(issues)` is defined but never referenced.
+- **Lines 1–25**: The Zod schemas (`locationPayloadSchema`, `joinRoomSchema`, `leaveRoomSchema`, `messageSchema`) and `MESSAGE_SIZE_LIMITS` are correct and should be preserved.
+
+## Required behavior
+- `validateMessage` exported once, accepting `string | unknown`.
+- Returns `{ ok: true, data }` where `data` matches the Zod discriminated union.
+- Returns `{ ok: false, error: string }` for: invalid JSON, oversized messages (string input only), schema validation failures.
+- `buildError` exported for external use.
+- All Zod schemas and size limits preserved exactly as-is.
+
+## Constraints
+- Do not change any file other than `src/validator.js`.
+- Do not add new npm dependencies.
+- Do not modify any test file.
+- The byte size check must use `Buffer.byteLength(raw, "utf8")` against the original raw string, not the parsed object.
+- The `MESSAGE_SIZE_LIMITS` object must remain exactly as defined (512/256/256).
+- Schema validation errors should be human-readable, joined with `"; "`.
+
+## Acceptance criteria
+- [ ] `src/validator.js` exports exactly one `validateMessage` function
+- [ ] No `SyntaxError` when importing `src/validator.js`
+- [ ] `npm run lint` passes
+- [ ] `validator.test.js` passes: all location_update, join_room, leave_room, and malformed JSON tests green
+- [ ] `message-size-limits.test.js` passes: all per-type size limit tests green
+- [ ] `integration.test.js` passes: valid messages route through validator → room manager, invalid messages are rejected
+- [ ] `validateMessage("not json")` returns `{ ok: false, error: "Invalid JSON" }`
+- [ ] `validateMessage({ type: "location_update", payload: { latitude: 40, longitude: -74 } })` returns `{ ok: true, data: ... }`
+
+## Out of scope
+- Changes to `server.js`, `auth.js`, `room-manager.js`, or any test file.
+- Adding new message types to the schema.
+- Changing the Zod schema definitions or size limits.
+
+## Hints and references
+- The `isString` check in the second `validateMessage` was checking `typeof raw === "string"` before the JSON parse, to decide whether to apply byte size limits. After parsing, you no longer have the original string to measure — so the size check must happen **before** parsing, using the raw input directly.
+- The `parseJSON` helper should attempt `JSON.parse` for strings and pass objects through, returning `{ ok: true, data }` or `{ ok: false, error: "Invalid JSON" }`.
+- The `formatIssue` that was referenced was likely meant to be a single-issue formatter; `buildError` already does this for arrays. Use `buildError(result.error.issues)` for the Zod failure path.

issues/issue-03-rate-limiter-memory-leak.md

@@ -0,0 +1,72 @@
+## Title
+Eliminate the unbounded memory leak in sliding-window rate limiters by implementing time-bucketed eviction with amortized O(1) cleanup
+
+## Difficulty
+10/10 — Expert. Estimated effort: 3–4 days for a senior engineer.
+
+## Context
+Both `src/rate-limiter.js` (per-message, 1-second window) and `src/conn-rate-limiter.js` (per-IP connection, 60-second window) store a `Map<string, number[]>` of timestamps. Stale entries are only evicted during an active `check()` call for that specific key — if a client connects once and never reconnects, or sends a burst and disconnects, their entry persists in the Map forever. In a fleet-tracking deployment with 10,000+ devices each sending 1 update/second, and with device reconnections creating new client IDs, this is an unbounded O(n) memory leak measured in megabytes per hour.
+
+The `remove()` method on `createRateLimiter` exists but is never called from `server.js` (which itself is broken and doesn't wire up disconnect cleanup for the per-message rate limiter). Even if it were called, `createConnRateLimiter` has no `remove()` method at all.
+
+This issue is **not** about fixing `server.js` — it is about fixing the rate limiters themselves so they are safe by construction, regardless of whether the caller remembers to call `remove()`.
+
+## Problem statement
+Implement bounded, self-cleaning sliding-window rate limiters for both modules that:
+
+1. **Guarantee memory is bounded**: No rate limiter instance should hold more than `K` entries where `K` is proportional to the number of active clients (not the historical total).
+2. **Evict stale entries proactively**: Entries older than the window (1 second for per-message, 60 seconds for per-IP) must be evicted without requiring a `check()` call for that key.
+3. **Maintain O(1) amortized `check()`**: The per-call cost must not degrade below O(1) amortized, even with eviction running.
+4. **Expose cleanup methods**: Both limiters must expose a `cleanup()` or `evict()` method that removes all stale entries, callable from a periodic timer or disconnect handler.
+5. **Expose metrics**: Both limiters must expose `size` (current entry count) for observability.
+
+## Current behavior
+`src/rate-limiter.js` (lines 34–42):
+```js
+let timestamps = windows.get(clientId);
+if (!timestamps) {
+  timestamps = [];
+  windows.set(clientId, timestamps);
+}
+while (timestamps.length > 0 && timestamps[0] <= cutoff) {
+  timestamps.shift();  // O(n) shift on each check
+}
+```
+The `shift()` is O(n) in the worst case. The Map only shrinks when `check()` is called for that specific key. `remove()` exists but is never called in production.
+
+`src/conn-rate-limiter.js` (lines 14–22): Identical pattern, identical problems, no `remove()` method.
+
+## Required behavior
+- Both `createRateLimiter` and `createConnRateLimiter` must not accumulate unbounded entries.
+- Each must expose `cleanup()` that iterates and removes entries where the oldest timestamp is older than the window.
+- Each must expose `size` getter returning the current Map size.
+- The `shift()` pattern must be replaced with a more efficient approach (e.g., ring buffer, or batch cleanup that replaces the array rather than shifting).
+- Memory usage must be measurable: a test that creates 100K entries, calls `cleanup()`, and verifies the Map shrinks to ≤ the number of entries with recent timestamps.
+
+## Constraints
+- Do not change the public API: `check(clientId)` and `remove(clientId)` on `createRateLimiter`, `check(ip)` on `createConnRateLimiter`.
+- Do not add new npm dependencies.
+- Do not modify `server.js` or any test file (except adding new test files if needed for the new behavior).
+- The sliding window semantics must remain identical: `maxPerSecond` messages in a 1-second window for per-message; `maxPerMinute` connections in a 60-second window for per-IP.
+- `check()` must still record the timestamp on success and return `false` on limit exceeded — no behavior change for callers.
+
+## Acceptance criteria
+- [ ] `createRateLimiter(5).check("c1")` returns `true` for 5 calls and `false` on the 6th (existing behavior preserved)
+- [ ] `createConnRateLimiter(3).check("1.1.1.1")` returns `true` for 3 calls and `false` on the 4th (existing behavior preserved)
+- [ ] After inserting 100K entries with old timestamps, calling `cleanup()` reduces Map size to ≤ entries with timestamps within the window
+- [ ] `size` getter returns current entry count
+- [ ] `rate-limiter.test.js` passes (all 7 tests)
+- [ ] `conn-rate-limiter.test.js` passes (all 4 createConnRateLimiter tests — skip the server tests)
+- [ ] Memory test: a test that calls `check()` for 50K unique keys, waits >1 second, calls `cleanup()`, and asserts `size` dropped by ≥ 90%
+- [ ] No `O(n)` `Array.shift()` in hot path (lint or code review)
+
+## Out of scope
+- Changes to `server.js`, `auth.js`, `validator.js`, `room-manager.js`, `index.js`, or `logger.js`.
+- Fixing the disconnect cleanup wiring in `server.js` (that is issue 1's responsibility).
+- Implementing token-bucket or leaky-bucket algorithms — this is specifically about fixing the sliding-window implementation.
+
+## Hints and references
+- A ring buffer (fixed-size circular array with head/tail pointers) replaces `Array.shift()` with an O(1) pointer bump, and `cleanup()` becomes a single pointer advance to the first non-expired entry.
+- Alternatively, batch-replace the array: instead of `shift()` one at a time, after `check()` completes, filter the array in one pass and replace the Map value. This is O(k) where k is the number of expired entries, amortized over all `check()` calls in that window.
+- The `cleanup()` method is useful for a periodic `setInterval` in `server.js` (e.g., every 10 seconds) to prevent unbounded growth even for keys that are never checked again.
+- Consider: what happens if `cleanup()` runs concurrently with `check()` on the same key? In Node.js single-threaded runtime this cannot happen within a single tick, but if `cleanup()` is async-aware or yields, it could. Keep it synchronous and atomic per tick.