From 20cb42d202d341c86c4289ae96e6af8d5eda0c4e Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Sat, 20 Jun 2026 09:31:14 -0400 Subject: [PATCH] COD-135 re-drive lost input ACK on a live WebSocket (durable-delivery gap) A reliable-input frame could be stranded forever if its server ACK ({t:'ia',seq}) was lost while the WebSocket kept delivering other output. _drainSession's WS fast path skips records with sentAt!==0, and after COD-134 the sweep only force-closes a *silent* socket -- so a lost ACK on an otherwise-live socket (stale && !silent) was never re-sent. _redeliverSweep now, for an active-WS session whose oldest unacked frame is stale but the socket is NOT silent, resets sentAt=0 on every stale unacked frame and lets the existing _drainSession re-drive them over the live socket (server dedups by seq). The stale && silent force-close remains the fallback for a genuinely half-open socket. Restores the exactly-once recovery guarantee without reintroducing the flap. Tests: new failing-first COD-135 cases in test/input-send-order.test.ts (re-drive on live socket; leave not-yet-stale alone; keep stale+silent force-close). 18/18 across input-send-order + reliable-input-dedup + ws-reconnect-plan; tsc 0, frontend-syntax, build all clean. --- src/web/public/app.js | 12 ++++++ test/input-send-order.test.ts | 75 +++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+) diff --git a/src/web/public/app.js b/src/web/public/app.js index 2f1472f3..ccec5458 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -2292,6 +2292,18 @@ class CodemanApp { } continue; } + if (stale) { + // Stale but the socket is still delivering output: the ACK was lost, + // not the connection. Force-closing isn't warranted (the link is fine), + // but the fast path skips anything with sentAt!==0, so the stranded + // frame would never re-send. Reset sentAt=0 on every stale unacked + // frame so the _drainSession below re-drives them over the live socket + // (server dedups by seq, so a re-sent lost-ACK frame is harmless). + // Frames sent recently (not yet stale) are left untouched. + for (const rec of list) { + if (rec.sentAt && Date.now() - rec.sentAt > this._reliableAckTimeoutMs) rec.sentAt = 0; + } + } } this._drainSession(sessionId); } diff --git a/test/input-send-order.test.ts b/test/input-send-order.test.ts index cc7c6294..1644f37d 100644 --- a/test/input-send-order.test.ts +++ b/test/input-send-order.test.ts @@ -157,3 +157,78 @@ describe('durable input delivery — send ordering', () => { expect(app._pendingDeliveries.get('session-1')).toBeUndefined(); }); }); + +// COD-135 — durable redelivery sweep when an ACK is lost. +type RedriveApp = App & { + _redeliverSweep: () => void; + _reliableAckTimeoutMs: number; + _wsLastRecvAt: number; +}; + +describe('durable input delivery — _redeliverSweep ACK-loss recovery (COD-135)', () => { + it('re-drives a stale unacked frame over a STILL-LIVE socket (lost ACK, not silent)', () => { + const app = makeApp() as RedriveApp; + const frames: Frame[] = []; + const close = vi.fn(); + app._ws = { readyState: 1, send: (d: string) => frames.push(JSON.parse(d)), close } as never; + app._wsSessionId = 'session-1'; + app._reliableAckTimeoutMs = 4000; + + // Frame sent once over the open socket; ACK never arrives. + app._sendInputAsync('session-1', 'a'); + expect(frames.map((f) => f.d)).toEqual(['a']); + + // ACK is lost, but the socket KEEPS receiving output → it is NOT silent. + // Backdate the send so the frame is stale; keep recv timestamp fresh. + const list = app._pendingDeliveries.get('session-1')!; + list[0].sentAt = Date.now() - (app._reliableAckTimeoutMs + 1000); + app._wsLastRecvAt = Date.now(); + + app._redeliverSweep(); + + // The stale frame must be re-sent over the live socket (a second send), + // and the socket must NOT be force-closed (it's alive, just the ACK was lost). + expect(frames.map((f) => f.d)).toEqual(['a', 'a']); + expect(close).not.toHaveBeenCalled(); + expect(app._pendingDeliveries.get('session-1')).toHaveLength(1); + }); + + it('does NOT re-drive a not-yet-stale frame (sent recently)', () => { + const app = makeApp() as RedriveApp; + const frames: Frame[] = []; + const close = vi.fn(); + app._ws = { readyState: 1, send: (d: string) => frames.push(JSON.parse(d)), close } as never; + app._wsSessionId = 'session-1'; + app._reliableAckTimeoutMs = 4000; + + app._sendInputAsync('session-1', 'a'); + app._wsLastRecvAt = Date.now(); // not silent + + // sentAt is fresh (just sent) → below the stale threshold → leave it alone. + app._redeliverSweep(); + + expect(frames.map((f) => f.d)).toEqual(['a']); // no second send + expect(close).not.toHaveBeenCalled(); + }); + + it('force-closes the socket when stale AND silent (half-open — COD-134 fallback preserved)', () => { + const app = makeApp() as RedriveApp; + const frames: Frame[] = []; + const close = vi.fn(); + app._ws = { readyState: 1, send: (d: string) => frames.push(JSON.parse(d)), close } as never; + app._wsSessionId = 'session-1'; + app._reliableAckTimeoutMs = 4000; + + app._sendInputAsync('session-1', 'a'); + const list = app._pendingDeliveries.get('session-1')!; + list[0].sentAt = Date.now() - (app._reliableAckTimeoutMs + 1000); // stale + app._wsLastRecvAt = Date.now() - (app._reliableAckTimeoutMs + 1000); // silent + + app._redeliverSweep(); + + // Half-open socket never recovers on its own → force-close to reconnect. + // It must NOT have re-sent over the dead socket. + expect(close).toHaveBeenCalledTimes(1); + expect(frames.map((f) => f.d)).toEqual(['a']); + }); +});