From c1c971ee47049ed77a268acaf7aeff59edc12891 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Apr 2026 09:18:45 +0000 Subject: [PATCH] fix(stream): commit seq after handler success, halve fence fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Warning: lastSeq was advanced immediately after the dedupe check, before the event handler ran. If a handler threw (e.g., chat:tags fetch failure), the seq was already marked consumed and a subsequent replay would skip it — silent data loss for side-channel events. Move the seq commit to after the handler body completes. If the handler throws, control exits via the exception and the commit never runs, so the next replay resends that seq. Use max() against the current stored value so a concurrent higher-seq handler that committed during our awaits isn't clobbered. Warning: fence fallback timeout was 10s, which is a visible live- stream stall when a peer backend silently drops the resume-stream request. Halved to 5s — still longer than the 1s XRANGE read timeout (legitimate slow replays complete), but short enough that a missing ack from a mixed-version peer unfreezes the UI sooner. --- src/lib/components/chat/Chat.svelte | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/src/lib/components/chat/Chat.svelte b/src/lib/components/chat/Chat.svelte index 5258af6507..20b70faa59 100644 --- a/src/lib/components/chat/Chat.svelte +++ b/src/lib/components/chat/Chat.svelte @@ -464,14 +464,17 @@ return; } - // Dedupe. + // Dedupe check only — advance lastSeq at the end of the + // handler, not here. If a downstream handler throws (e.g., + // chat:tags fetch fails), leaving lastSeq unchanged lets + // a subsequent replay retry that seq instead of silently + // skipping it. const incomingSeq = typeof event?.seq === 'number' ? event.seq : null; if (incomingSeq !== null) { const lastSeq = resumeSeqByMessageId.get(event.message_id) ?? 0; if (incomingSeq <= lastSeq) { return; } - resumeSeqByMessageId.set(event.message_id, incomingSeq); } const type = event?.data?.type ?? null; @@ -628,6 +631,17 @@ } history.messages[event.message_id] = message; + + // Handler completed without throwing — safe to commit the + // seq now. Use max() against the current value so a + // concurrent higher-seq handler that committed during our + // awaits doesn't get clobbered. + if (incomingSeq !== null) { + const current = resumeSeqByMessageId.get(event.message_id) ?? 0; + if (incomingSeq > current) { + resumeSeqByMessageId.set(event.message_id, incomingSeq); + } + } } } else { // Non-active chat completion: queue stays in the global store. @@ -635,8 +649,12 @@ } }; - // Fallback timer in case the replay ack never arrives. - const RESUME_FENCE_TIMEOUT_MS = 10000; + // Fallback timer in case the replay ack never arrives. 5s balances: + // a legitimately slow Redis XRANGE at RESUME_STREAM_READ_TIMEOUT_SEC + // (1s default), versus how long the UI should hold live frames if + // a peer backend silently drops the request (mixed-version rollout, + // handler not registered, etc). + const RESUME_FENCE_TIMEOUT_MS = 5000; const resumeFenceTimerByMessageId = new Map(); // Active request_id per message — lets us ignore stale replay // responses from superseded requests (reconnect churn).