COD-137 scope WS per-session limit by clientId (fix spurious 4008 on reconnect)

MAX_WS_PER_SESSION was gated by a bare Map<sessionId,number> counter,
incremented on upgrade and decremented only on the old socket's async
close. A client that dropped and immediately reconnected could land its
new upgrade before the old socket's close fired, briefly over-counting and
tripping a spurious 4008 (-> HTTP fallback). The limit also counted raw
sockets, so a reconnecting client consumed a new slot instead of its own.

Replace the counter with WsConnectionRegistry (new pure, unit-tested module)
that tracks live sockets per session keyed by clientId. A same-cid upgrade
SUPERSEDES its own socket (evicts the stale one with close 4010, reuses the
slot, no net count change) -> a reconnect can never be rejected by the cap.
The reliable-input protocol (shouldApplyInput(cid,seq)) already assumes one
logical client per cid per session, so same-cid eviction is principled, not
a regression of multi-tab (which already collides on seq). Slots are freed
EAGERLY on error/terminate, not just async close; close is identity-matched
so a superseded socket's late close is a no-op. cid-less upgrades are
admitted anonymously up to the cap and never evict (backward-compat).
Client sends cid on the WS upgrade URL (?cid=, encoded, omitted if absent).

Tests: ws-connection-registry.test.ts (reconnect-reclaim at cap, rejects
N+1th distinct, eager-terminate frees slot, cid-less up-to-limit + no-evict,
late-close-no-evict, per-session isolation) + route integration in
ws-routes.test.ts (real upgrade through the cap). 45/45 across registry +
ws-routes + input-send-order + ws-reconnect-plan; tsc 0, build, prettier,
frontend-syntax clean.
This commit is contained in:
Aamer Akhter
2026-07-10 14:39:01 -04:00
parent 20cb42d202
commit 4ab89f9a4e
5 changed files with 471 additions and 175 deletions
+23
View File
@@ -491,6 +491,29 @@ describe('ws-routes', () => {
for (const ws of connections) ws.close();
}
});
it('reconnecting client (same cid) is admitted at the cap instead of 4008 (COD-137)', async () => {
const connections: WebSocket[] = [];
try {
// Fill all 5 slots with DISTINCT clients, one of which is "alice".
for (const c of ['alice', 'b', 'c', 'd', 'e']) {
connections.push(await connectWs(`/ws/sessions/ws-test-session/terminal?cid=${c}`));
}
// Alice reconnects WHILE her old socket is still registered (the
// over-count window). This must reclaim her slot, not hit the cap.
const aliceNew = await connectWs('/ws/sessions/ws-test-session/terminal?cid=alice');
connections.push(aliceNew);
// Sanity: the reconnected socket is live and usable.
ctx._session.emit('terminal', 'reconnected-ok');
const msg = (await nextMessage(aliceNew)) as { t: string; d: string };
expect(msg.t).toBe('o');
expect(msg.d).toContain('reconnected-ok');
} finally {
for (const ws of connections) ws.close();
}
});
});
// ========== Heartbeat ==========
+112
View File
@@ -0,0 +1,112 @@
/**
* @fileoverview Unit tests for WsConnectionRegistry (COD-137).
*
* The registry is the pure decision unit extracted out of ws-routes.ts so the
* connection-limit / clientId-eviction logic is testable without driving real
* WebSocket upgrades. Uses plain fake sockets (identity only).
*
* @dependency src/web/ws-connection-registry.ts
*/
import { describe, it, expect } from 'vitest';
import { WsConnectionRegistry } from '../src/web/ws-connection-registry.js';
/** Fake socket — registry only compares identity, so any object works. */
const sock = (label: string) => ({ readyState: 1, label });
describe('WsConnectionRegistry', () => {
it('reconnecting client (same cid) reclaims its slot instead of being rejected at the limit', () => {
const reg = new WsConnectionRegistry(5);
// Fill all 5 slots with distinct clients, one of which is "alice".
for (const c of ['alice', 'b', 'c', 'd', 'e']) {
expect(reg.register('s1', c, sock(c)).admitted).toBe(true);
}
expect(reg.liveCount('s1')).toBe(5);
// Alice's new upgrade lands BEFORE her old socket's async close fires.
const aliceNew = sock('alice-new');
const res = reg.register('s1', 'alice', aliceNew);
expect(res.admitted).toBe(true); // NOT a spurious 4008
expect(res.evictedSocket).toBeDefined(); // old alice socket handed back to close
expect(reg.liveCount('s1')).toBe(5); // slot reused, not double-counted
});
it('still rejects a genuine (N+1)th DISTINCT client', () => {
const reg = new WsConnectionRegistry(5);
for (const c of ['a', 'b', 'c', 'd', 'e']) {
expect(reg.register('s1', c, sock(c)).admitted).toBe(true);
}
const sixth = reg.register('s1', 'f', sock('f'));
expect(sixth.admitted).toBe(false);
expect(sixth.evictedSocket).toBeUndefined();
expect(reg.liveCount('s1')).toBe(5);
});
it('eager removal on terminate frees a slot immediately', () => {
const reg = new WsConnectionRegistry(5);
const sockets = ['a', 'b', 'c', 'd', 'e'].map((c) => {
const s = sock(c);
reg.register('s1', c, s);
return [c, s] as const;
});
expect(reg.register('s1', 'f', sock('f')).admitted).toBe(false);
// Eagerly unregister one (simulating terminate/error, not async close).
reg.unregister('s1', sockets[0][1]);
expect(reg.liveCount('s1')).toBe(4);
// Now a brand-new distinct client is admitted.
expect(reg.register('s1', 'f', sock('f')).admitted).toBe(true);
expect(reg.liveCount('s1')).toBe(5);
});
it('cid-less upgrades are admitted up to the limit and never evict a keyed client', () => {
const reg = new WsConnectionRegistry(5);
const keyed = sock('keyed');
reg.register('s1', 'keyed', keyed);
// Four anonymous upgrades fill the rest of the cap.
for (let i = 0; i < 4; i++) {
const res = reg.register('s1', null, sock(`anon${i}`));
expect(res.admitted).toBe(true);
expect(res.evictedSocket).toBeUndefined(); // never evicts the keyed client
}
expect(reg.liveCount('s1')).toBe(5);
// 6th anonymous is rejected — anonymous sockets count toward the cap.
expect(reg.register('s1', null, sock('anon-extra')).admitted).toBe(false);
// The keyed client is untouched: a same-cid reconnect still reclaims.
const keyedNew = sock('keyed-new');
const res = reg.register('s1', 'keyed', keyedNew);
expect(res.admitted).toBe(true);
expect(res.evictedSocket).toBe(keyed);
});
it('late close of a superseded socket does not evict the reconnected one', () => {
const reg = new WsConnectionRegistry(5);
const old = sock('old');
reg.register('s1', 'alice', old);
const fresh = sock('fresh');
reg.register('s1', 'alice', fresh); // supersede
// The stale socket's async close arrives late — must NOT remove fresh.
reg.unregister('s1', old);
expect(reg.liveCount('s1')).toBe(1);
// Fresh is still the live entry: another reconnect evicts fresh, not old.
const fresher = sock('fresher');
expect(reg.register('s1', 'alice', fresher).evictedSocket).toBe(fresh);
});
it('isolates counts per session', () => {
const reg = new WsConnectionRegistry(2);
reg.register('s1', 'a', sock('a'));
reg.register('s1', 'b', sock('b'));
expect(reg.register('s1', 'c', sock('c')).admitted).toBe(false);
// s2 has its own budget.
expect(reg.register('s2', 'a', sock('a2')).admitted).toBe(true);
expect(reg.liveCount('s2')).toBe(1);
});
});